From 18053b80c8f2f0a7b13a887e1b440b4dab434481 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:02:34 +0000 Subject: [PATCH 1/6] feat(azure): reuse shared secret planning and hosted delivery --- build/secrets-publisher/Dockerfile | 5 + cmd/deploy/gitops.go | 30 +- cmd/deploy/secrets.go | 29 ++ cmd/deploy/secrets_input.go | 131 +++++++++ cmd/deploy/secrets_input_test.go | 72 +++++ cmd/environment/configuration_profile_test.go | 41 +++ cmd/environment/import.go | 76 +++-- docs/cli-reference.md | 48 +++- docs/commands.md | 13 + docs/deployment-secret-safety.md | 70 ++++- docs/module-manifests.md | 34 ++- go.mod | 10 + go.sum | 26 ++ pkg/delivery/signing/azure_key_vault.go | 179 ++++++++++++ pkg/delivery/signing/azure_key_vault_test.go | 257 +++++++++++++++++ pkg/delivery/signing/cloud_kms.go | 32 ++- pkg/delivery/signing/cloud_kms_test.go | 29 ++ pkg/delivery/signing/signing.go | 2 +- pkg/delivery/signing/vault_transit.go | 13 +- pkg/deploysecrets/azure.go | 265 ++++++++++++++++++ pkg/deploysecrets/azure_test.go | 258 +++++++++++++++++ pkg/deploysecrets/plan.go | 205 ++++++++++++-- pkg/deploysecrets/plan_test.go | 8 +- pkg/deploysecrets/retained_material_test.go | 84 ++++++ pkg/deploysecrets/rotate_test.go | 4 +- pkg/deploysecrets/store.go | 15 +- pkg/deploysecrets/supplied_test.go | 95 +++++++ pkg/environments/environment.go | 14 +- pkg/environments/environment_test.go | 18 +- pkg/environments/workspace.go | 15 +- pkg/gitops/cell.go | 25 +- pkg/gitops/cell_test.go | 24 ++ pkg/gitops/configuration_projection.go | 4 +- pkg/gitops/configuration_projection_test.go | 43 +++ pkg/gitops/configuration_templates.go | 2 +- pkg/gitops/dev.go | 2 +- pkg/gitops/frontend_payload.go | 11 +- pkg/gitops/frontend_publish_test.go | 2 +- pkg/gitops/orchestrate.go | 4 +- pkg/gitops/publish.go | 1 + pkg/gitops/secret_projection.go | 2 +- pkg/gitops/secret_projection_test.go | 29 ++ pkg/gitops/solution.go | 2 +- pkg/internal/azureauth/credential.go | 50 ++++ pkg/internal/azureauth/credential_test.go | 47 ++++ pkg/orchestration/build_plan.go | 13 +- pkg/orchestration/image_cache.go | 10 +- pkg/orchestration/image_cache_test.go | 6 + pkg/orchestration/registry_mirror.go | 11 +- pkg/solutionrun/frontend_build.go | 19 +- 50 files changed, 2264 insertions(+), 121 deletions(-) create mode 100644 build/secrets-publisher/Dockerfile create mode 100644 cmd/deploy/secrets_input.go create mode 100644 cmd/deploy/secrets_input_test.go create mode 100644 pkg/delivery/signing/azure_key_vault.go create mode 100644 pkg/delivery/signing/azure_key_vault_test.go create mode 100644 pkg/deploysecrets/azure.go create mode 100644 pkg/deploysecrets/azure_test.go create mode 100644 pkg/deploysecrets/retained_material_test.go create mode 100644 pkg/deploysecrets/supplied_test.go create mode 100644 pkg/internal/azureauth/credential.go create mode 100644 pkg/internal/azureauth/credential_test.go diff --git a/build/secrets-publisher/Dockerfile b/build/secrets-publisher/Dockerfile new file mode 100644 index 000000000..3af6ab673 --- /dev/null +++ b/build/secrets-publisher/Dockerfile @@ -0,0 +1,5 @@ +# Package the existing CLI for infrastructure-owned publisher Jobs. +# The operator builds codefly from the exact source revision before building this image. +FROM gcr.io/distroless/cc-debian12:nonroot@sha256:9dac0a79194e45a7da0158a9c6da57b217585af0786db3845d1f0ec1a0dd182f +COPY --chmod=0555 codefly /codefly +ENTRYPOINT ["/codefly"] diff --git a/cmd/deploy/gitops.go b/cmd/deploy/gitops.go index bb235a05b..d652a8897 100644 --- a/cmd/deploy/gitops.go +++ b/cmd/deploy/gitops.go @@ -34,6 +34,32 @@ var GitOpsCmd = &cobra.Command{ Short: "Render, publish, observe, and recover reviewed GitOps promotions", } +var gitOpsCellCmd = &cobra.Command{ + Use: "cell", + Short: "Derive the cell file from published module trees without rebuilding images", + Args: cobra.NoArgs, + RunE: func(_ *cobra.Command, _ []string) error { + ctx, done := common.NewContext() + defer done() + if err := common.ResolvePinnedModules(ctx); err != nil { + return err + } + workspace, err := common.LoadWorkspace(ctx) + if err != nil { + return err + } + env, err := orchestration.SelectEnvironment(workspace, gitOpsEnv) + if err != nil { + return err + } + cell, err := gitops.RenderPublishedCell(ctx, workspace, env) + if err == nil { + printCell(&cell) + } + return err + }, +} + var gitOpsRenderCmd = &cobra.Command{ Use: "render [module]", Short: "Render and validate a module-owned manifest tree", @@ -614,8 +640,8 @@ var ( ) func init() { - GitOpsCmd.AddCommand(gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd) - for _, command := range []*cobra.Command{gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd} { + GitOpsCmd.AddCommand(gitOpsCellCmd, gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd) + for _, command := range []*cobra.Command{gitOpsCellCmd, gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd} { command.Flags().StringVar(&gitOpsEnv, "env", "local", "Environment to promote") } // The delivery verbs — the ones that render a manifest tree from the diff --git a/cmd/deploy/secrets.go b/cmd/deploy/secrets.go index 2f204b9a4..0ede8155a 100644 --- a/cmd/deploy/secrets.go +++ b/cmd/deploy/secrets.go @@ -27,6 +27,10 @@ var ( secretsAllowMissing bool secretsYes bool secretsModules []string + secretsMintKeys []string + secretsExportInput string + secretsInput string + secretsSupplied string secretsRotate []string ) @@ -56,6 +60,11 @@ not. Both are reported. Values are never printed: the plan names keys and sources only. +--supplied names a JSON object of secret key to value for the required ones: a +supplied value is written where nothing holds the key, never over a stored value +and never in place of a declared generator. An empty value is accepted only for +a key service-secrets.may-be-empty declares. + --dry-run prints the plan and writes nothing. --metadata-only additionally never reads a stored value: it knows which remote keys exist, not what they hold. @@ -78,6 +87,12 @@ restart its consumers. An identifier or signing key is never rotated.`, RunE: func(cmd *cobra.Command, _ []string) error { ctx, done := common.NewContext() defer done() + if secretsInput != "" { + return runSecretsInput(ctx, cmd) + } + if secretsExportInput != "" && (len(secretsModules) != 0 || len(secretsMintKeys) != 0 || secretsAllowMissing || secretsMetadataOnly || secretsSupplied != "") { + return fmt.Errorf("--export-input exports the full render without module, mint, allow-missing, metadata-only or supplied options") + } if secretsMetadataOnly && !secretsDryRun { return fmt.Errorf("--metadata-only plans without reading the store, so it cannot be applied: pass --dry-run") } @@ -105,6 +120,13 @@ restart its consumers. An identifier or signing key is never rotated.`, return fmt.Errorf("--module %s: not a module rendered for %s (rendered: %s)", module, env.Name, strings.Join(rendered.Modules, ", ")) } } + if secretsExportInput != "" { + return exportSecretsInput(secretsExportInput, env, rendered, workspaceServiceUniques(ctx, workspace, env)) + } + supplied, err := readSuppliedValues(secretsSupplied, rendered, env.ServiceSecrets.MayBeEmpty) + if err != nil { + return err + } groups := renderedByStore(rendered) rotate, err := rotationKeys(rendered, groups, secretsRotate) if err != nil { @@ -134,6 +156,8 @@ restart its consumers. An identifier or signing key is never rotated.`, ReadPayloads: !secretsMetadataOnly, Modules: secretsModules, MayBeEmpty: env.ServiceSecrets.MayBeEmpty, + MintKeys: secretsMintKeys, + Supplied: supplied, Configured: configured, Rotate: readBy(group, rotate), }) @@ -326,5 +350,10 @@ func init() { SecretsCmd.Flags().BoolVar(&secretsAllowMissing, "allow-missing", false, "Write what can be resolved even when some properties must still be supplied") SecretsCmd.Flags().BoolVarP(&secretsYes, "yes", "y", false, "Write without an interactive confirmation") SecretsCmd.Flags().StringSliceVar(&secretsModules, "module", nil, "Plan only these modules' remote keys (comma-separated or repeated)") + SecretsCmd.Flags().StringSliceVar(&secretsMintKeys, "mint", nil, "Acknowledge first creation of these signing secret keys (comma-separated or repeated); never rotate existing keys") + SecretsCmd.Flags().StringVar(&secretsExportInput, "export-input", "", "Export declarative planner inputs for execution by an infrastructure-owned publisher; reads no stored values") + SecretsCmd.Flags().StringVar(&secretsInput, "input", "", "Run exported planner inputs under an infrastructure-bound Azure workload identity") + SecretsCmd.Flags().StringVar(&secretsSupplied, "supplied", "", "JSON object of operator values by secret key, for keys nothing stores or generates; never printed") + SecretsCmd.MarkFlagsMutuallyExclusive("export-input", "input") SecretsCmd.Flags().StringSliceVar(&secretsRotate, "rotate", nil, "Replace a generated key's stored value and every stored value derived from it: //:, :, or the CODEFLY__ key (repeatable)") } diff --git a/cmd/deploy/secrets_input.go b/cmd/deploy/secrets_input.go new file mode 100644 index 000000000..f5c55c014 --- /dev/null +++ b/cmd/deploy/secrets_input.go @@ -0,0 +1,131 @@ +package deploy + +import ( + "context" + "encoding/json" + "errors" + "io" + "os" + + "github.com/codefly-dev/cli/pkg/deploysecrets" + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/cli/pkg/gitops" + "github.com/spf13/cobra" +) + +// Only declarations travel to the publisher. Existing values and generated +// material are resolved by the normal planner inside its authorized execution. +type secretsInputDocument struct { + Schema string + Environment string + Rendered gitops.RenderedEnvironment + Generators []environments.EnvironmentSecretGenerator + Services []string + MayBeEmpty []string +} + +const secretsInputSchema = "codefly.secrets-input/v1" + +func exportSecretsInput(path string, env *environments.Environment, rendered gitops.RenderedEnvironment, services []string) error { + document := secretsInputDocument{Schema: secretsInputSchema, Environment: env.Name, + Rendered: rendered, Generators: env.ServiceSecrets.Generate, Services: services, + MayBeEmpty: env.ServiceSecrets.MayBeEmpty} + file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return errors.New("cannot exclusively create secret planner input file") + } + err = json.NewEncoder(file).Encode(document) + closeErr := file.Close() + return errors.Join(err, closeErr) +} + +func readSecretsInput(input io.Reader, storeName string) (secretsInputDocument, error) { + var document secretsInputDocument + decoder := json.NewDecoder(io.LimitReader(input, 1024*1024+1)) + decoder.DisallowUnknownFields() + if decoder.Decode(&document) != nil || decoder.Decode(new(any)) != io.EOF || + document.Schema != secretsInputSchema || document.Environment == "" || + storeName == "" || len(document.Rendered.Secrets) == 0 { + return document, errors.New("invalid secret planner input; contents withheld") + } + for _, secret := range document.Rendered.Secrets { + if secret.Store.Kind != "ClusterSecretStore" || secret.Store.Name != storeName { + return document, errors.New("planner input does not match the infrastructure-bound ClusterSecretStore") + } + } + declarations := environments.EnvironmentServiceSecrets{ + SecretStore: document.Rendered.Secrets[0].Store, Generate: document.Generators, + } + if declarations.Validate() != nil { + return document, errors.New("invalid secret generator declarations; contents withheld") + } + return document, nil +} + +func runSecretsInput(ctx context.Context, cmd *cobra.Command) error { + // The exported document cannot select its destination or impersonate a reader. + // Infrastructure binds these from the live store and its existing writer Job. + if os.Getenv("CODEFLY_AZURE_CREDENTIAL") != "workload" { + return errors.New("secret planner input execution requires Azure workload identity") + } + if secretsExportInput != "" || secretsMetadataOnly || len(secretsModules) != 0 || len(secretsMintKeys) != 0 || secretsAllowMissing { + return errors.New("secret planner input execution requires the complete input without mint or allow-missing") + } + file, err := os.Open(secretsInput) + if err != nil { + return errors.New("cannot open secret planner input") + } + defer file.Close() + info, err := file.Stat() + if err != nil || !info.Mode().IsRegular() || info.Size() > 1024*1024 { + return errors.New("secret planner input must be a regular file of at most 1 MiB") + } + document, err := readSecretsInput(file, os.Getenv("CODEFLY_SECRETS_STORE_NAME")) + if err != nil { + return err + } + supplied, err := readSuppliedValues(secretsSupplied, document.Rendered, document.MayBeEmpty) + if err != nil { + return err + } + store, err := deploysecrets.AzurePublisherStore(ctx, os.Getenv("CODEFLY_SECRETS_AZURE_VAULT_URL"), os.Getenv("AZURE_TENANT_ID")) + if err != nil { + return err + } + plan, err := deploysecrets.Build(ctx, &deploysecrets.Inputs{ + Rendered: document.Rendered, Generators: document.Generators, Services: document.Services, + MayBeEmpty: document.MayBeEmpty, Store: store, ReadPayloads: true, Supplied: supplied, + }) + if err != nil { + return err + } + printSecretsPlan(cmd.OutOrStdout(), document.Environment, document.Rendered, plan) + return finishSecretsPlan(ctx, plan, store, secretsDryRun, false, func() bool { return secretsYes }) +} + +// readSuppliedValues reads the operator's values for keys nothing stores or +// generates: one JSON object of secret key to string. Errors never quote the +// file, which holds credentials. +func readSuppliedValues(path string, rendered gitops.RenderedEnvironment, mayBeEmpty []string) (map[string]string, error) { + if path == "" { + return nil, nil + } + file, err := os.Open(path) + if err != nil { + return nil, errors.New("cannot open supplied values") + } + defer file.Close() + info, err := file.Stat() + if err != nil || !info.Mode().IsRegular() || info.Size() > 1024*1024 { + return nil, errors.New("supplied values must be a regular file of at most 1 MiB") + } + var supplied map[string]string + decoder := json.NewDecoder(io.LimitReader(file, 1024*1024+1)) + if decoder.Decode(&supplied) != nil || decoder.Decode(new(any)) != io.EOF || supplied == nil { + return nil, errors.New("supplied values must be one JSON object of secret key to string; contents withheld") + } + if err := deploysecrets.ValidateSupplied(rendered, supplied, mayBeEmpty); err != nil { + return nil, err + } + return supplied, nil +} diff --git a/cmd/deploy/secrets_input_test.go b/cmd/deploy/secrets_input_test.go new file mode 100644 index 000000000..cca8b52f1 --- /dev/null +++ b/cmd/deploy/secrets_input_test.go @@ -0,0 +1,72 @@ +package deploy + +import ( + "bytes" + "encoding/json" + "os" + "strings" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/cli/pkg/gitops" +) + +func TestSecretsInputBindsEveryStoreAndRejectsPayloadFields(t *testing.T) { + document := secretsInputDocument{Schema: secretsInputSchema, Environment: "azure-staging", + Rendered: gitops.RenderedEnvironment{Secrets: []gitops.RenderedServiceSecret{ + {Store: environments.EnvironmentSecretStoreReference{Kind: "ClusterSecretStore", Name: "cell-secrets"}}, + }}} + raw, err := json.Marshal(document) + if err != nil { + t.Fatal(err) + } + if _, err := readSecretsInput(bytes.NewReader(raw), "cell-secrets"); err != nil { + t.Fatal(err) + } + for _, name := range []string{"", "another-store"} { + if _, err := readSecretsInput(bytes.NewReader(raw), name); err == nil { + t.Fatal("accepted wrong store") + } + } + for _, changed := range []string{ + strings.Replace(string(raw), `"Schema":`, `"Values":{"token":"never-print"},"Schema":`, 1), + string(raw) + `{}`, strings.Replace(string(raw), secretsInputSchema, "unknown/v2", 1), + strings.Replace(string(raw), "ClusterSecretStore", "SecretStore", 1), + } { + if _, err := readSecretsInput(strings.NewReader(changed), "cell-secrets"); err == nil || strings.Contains(err.Error(), "never-print") { + t.Fatal("invalid input was accepted or payload leaked") + } + } +} + +func TestSuppliedValuesAreOneObjectAndNeverQuoted(t *testing.T) { + key := "CODEFLY__WORKSPACE_SECRET_CONFIGURATION__IDENTITY__IDENTITY_CLIENT_SECRET" + rendered := gitops.RenderedEnvironment{Secrets: []gitops.RenderedServiceSecret{{ + RemoteKey: "host-accounts", + Properties: []gitops.RenderedSecretProperty{{Property: key, Keys: []string{key}}}, + }}} + path := t.TempDir() + "/supplied.json" + write := func(content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + write(`{"` + key + `":"never-print"}`) + supplied, err := readSuppliedValues(path, rendered, nil) + if err != nil || supplied[key] != "never-print" { + t.Fatalf("readSuppliedValues = %v, %v", supplied, err) + } + for _, content := range []string{ + `["never-print"]`, `{"` + key + `":"never-print"}{}`, `{"` + key + `":1}`, `never-print`, + `{"CODEFLY__WORKSPACE_SECRET_CONFIGURATION__TYPO__X":"never-print"}`, + } { + write(content) + if _, err := readSuppliedValues(path, rendered, nil); err == nil || strings.Contains(err.Error(), "never-print") { + t.Fatalf("accepted %q or leaked it: %v", content, err) + } + } + if supplied, err := readSuppliedValues("", rendered, nil); err != nil || supplied != nil { + t.Fatalf("no file = %v, %v; want nothing", supplied, err) + } +} diff --git a/cmd/environment/configuration_profile_test.go b/cmd/environment/configuration_profile_test.go index 1c9f485fc..d619bf25a 100644 --- a/cmd/environment/configuration_profile_test.go +++ b/cmd/environment/configuration_profile_test.go @@ -1,6 +1,7 @@ package environment import ( + "bytes" "os" "path/filepath" "strings" @@ -11,6 +12,46 @@ import ( "github.com/stretchr/testify/require" ) +func TestImportProductProfileIntoExistingPlacement(t *testing.T) { + root := profileSource(t) + workspace := "name: acme\nlayout: modules\nenvironments:\n- name: azure\n namespace: acme\n service-identity:\n services:\n model/model:\n principal: applied-client\n" + require.NoError(t, os.WriteFile(filepath.Join(root, "workspace.codefly.yaml"), []byte(workspace), 0o644)) + opts := importOptions{configurationSource: "hosted", configurationProfiles: []string{"azure", "local"}, dryRun: true} + doImport(t, root, opts) + require.Equal(t, workspace, readFile(t, filepath.Join(root, "workspace.codefly.yaml"))) + _, err := os.Stat(filepath.Join(root, "configurations", "azure")) + require.True(t, os.IsNotExist(err)) + opts.dryRun = false + doImport(t, root, opts) + env := loadWorkspace(t, root).FindEnvironment("azure") + require.Equal(t, "hosted", env.ConfigurationSource) + require.Equal(t, []string{"azure", "local"}, env.ConfigurationProfiles) + require.Contains(t, readFile(t, filepath.Join(root, "workspace.codefly.yaml")), "principal: applied-client") + profile := filepath.Join(root, "configurations", "azure", configurations.ProfileDerivationFile) + require.Contains(t, readFile(t, profile), "derives-from: hosted") + // A later placement refresh keeps and regenerates the product selection. + require.NoError(t, os.Remove(profile)) + doImport(t, root, importOptions{}) + require.Contains(t, readFile(t, profile), "derives-from: hosted") +} + +func TestImportProductProfileRefusesCoordinateConflict(t *testing.T) { + for _, declared := range []string{ + `"configuration-source":"other"`, + `"configuration-profile":"other"`, + `"configuration-profiles":["other"]`, + } { + t.Run(declared, func(t *testing.T) { + root := profileSource(t) + workspace := "name: acme\nlayout: modules\n" + require.NoError(t, os.WriteFile(filepath.Join(root, "workspace.codefly.yaml"), []byte(workspace), 0o644)) + opts := importOptions{dir: root, envName: "azure", configurationSource: "hosted", configurationProfiles: []string{"azure", "local"}, contractData: []byte(`{"schema":"codefly/coordinate/v1","environment":{"name":"azure",` + declared + `}}`), stdout: &bytes.Buffer{}} + require.Error(t, runImport(t.Context(), &opts)) + require.Equal(t, workspace, readFile(t, filepath.Join(root, "workspace.codefly.yaml"))) + }) + } +} + func profileSource(t *testing.T) string { t.Helper() root := t.TempDir() diff --git a/cmd/environment/import.go b/cmd/environment/import.go index f94fb096a..396b05cfb 100644 --- a/cmd/environment/import.go +++ b/cmd/environment/import.go @@ -86,6 +86,8 @@ full delivery.intent.yaml is not interpreted.`, if err != nil { return err } + configurationSource, _ := cmd.Flags().GetString("configuration-source") + configurationProfiles, _ := cmd.Flags().GetStringSlice("configuration-profiles") productPath, err := cmd.Flags().GetString("product-intent") if err != nil { @@ -108,13 +110,15 @@ full delivery.intent.yaml is not interpreted.`, } opts := importOptions{ - dir: *dir, - envName: args[0], - contractData: data, - productIntentData: productData, - namespace: importNamespace, - namespaceSet: cmd.Flags().Changed("namespace"), - dryRun: importDryRun, + dir: *dir, + envName: args[0], + contractData: data, + productIntentData: productData, + namespace: importNamespace, + namespaceSet: cmd.Flags().Changed("namespace"), + dryRun: importDryRun, + configurationSource: configurationSource, + configurationProfiles: configurationProfiles, // UTC so the provenance timestamp does not vary by the operator's // machine timezone and churn the diff on re-import across a team. now: time.Now().UTC(), @@ -170,15 +174,17 @@ func readContract(stdin io.Reader, path string) ([]byte, error) { } type importOptions struct { - dir string - envName string - contractData []byte - productIntentData []byte - namespace string - namespaceSet bool - dryRun bool - now time.Time - stdout io.Writer + dir string + envName string + contractData []byte + productIntentData []byte + namespace string + namespaceSet bool + dryRun bool + now time.Time + stdout io.Writer + configurationSource string + configurationProfiles []string } // runImport parses the contract, merges its owned fields into the named @@ -202,6 +208,30 @@ func runImport(ctx context.Context, opts *importOptions) error { return err } declaredEnvironment := yamledit.MapValue(declaration, "environment") + // Product profiles are owned by the consuming workspace. A placement-only + // producer need not acquire product configuration to import its target. + if opts.configurationSource != "" { + if existing := contract.Environment.ConfigurationSource; existing != "" && existing != opts.configurationSource { + return fmt.Errorf("--configuration-source conflicts with the coordinate declaration") + } + contract.Environment.ConfigurationSource = opts.configurationSource + node, encodeErr := yamledit.Encode(opts.configurationSource) + if encodeErr != nil { + return encodeErr + } + yamledit.SetMapValue(declaredEnvironment, "configuration-source", node) + } + if len(opts.configurationProfiles) > 0 { + if contract.Environment.ConfigurationProfile != "" || len(contract.Environment.ConfigurationProfiles) > 0 { + return fmt.Errorf("--configuration-profiles cannot replace coordinate-declared profiles") + } + contract.Environment.ConfigurationProfiles = opts.configurationProfiles + node, encodeErr := yamledit.Encode(opts.configurationProfiles) + if encodeErr != nil { + return encodeErr + } + yamledit.SetMapValue(declaredEnvironment, "configuration-profiles", node) + } file := filepath.Join(opts.dir, resources.WorkspaceConfigurationName) original, err := os.ReadFile(file) @@ -267,14 +297,14 @@ func runImport(ctx context.Context, opts *importOptions) error { if err != nil { return err } - if _, err := environments.FromWorkspace(merged); err != nil { + if _, err = environments.FromWorkspace(merged); err != nil { return fmt.Errorf("merged environment declarations: %w", err) } + selected, err := environments.Select(merged, opts.envName) + if err != nil { + return err + } if intent != nil { - selected, err := environments.Select(merged, opts.envName) - if err != nil { - return err - } workspace, err := resources.LoadWorkspaceFromDir(ctx, opts.dir) if err != nil { return err @@ -286,7 +316,7 @@ func runImport(ctx context.Context, opts *importOptions) error { return fmt.Errorf("product intent: %w", err) } } - if err := materializeConfigurationProfile(ctx, opts.dir, env, opts.dryRun); err != nil { + if err := materializeConfigurationProfile(ctx, opts.dir, selected, opts.dryRun); err != nil { return err } if opts.dryRun { @@ -608,6 +638,8 @@ func stampProvenance(envNode *yaml.Node, contract *environments.CoordinateContra func init() { importCmd.Flags().String("coordinate-contract", "", "Path to a codefly/coordinate/v1 descriptor, or - for stdin") + importCmd.Flags().String("configuration-source", "", "Authored product profile inherited by the imported environment") + importCmd.Flags().StringSlice("configuration-profiles", nil, "Product configuration profile chain when absent from the coordinate declaration") importCmd.Flags().String("product-intent", "", "Authored product decisions: environments..exclude-services and service-secrets generate / may-be-empty") importCmd.Flags().String("cell-contract", "", "Former spelling of --coordinate-contract") _ = importCmd.Flags().MarkDeprecated("cell-contract", "use --coordinate-contract") diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 966ccb69e..91a762815 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2125,6 +2125,7 @@ codefly deploy gitops Subcommands: +- [`codefly deploy gitops cell`](#codefly-deploy-gitops-cell) - [`codefly deploy gitops observe`](#codefly-deploy-gitops-observe) - [`codefly deploy gitops plan`](#codefly-deploy-gitops-plan) - [`codefly deploy gitops publish`](#codefly-deploy-gitops-publish) @@ -2133,6 +2134,20 @@ Subcommands: - [`codefly deploy gitops rollback`](#codefly-deploy-gitops-rollback) - [`codefly deploy gitops snapshot`](#codefly-deploy-gitops-snapshot) +## `codefly deploy gitops cell` + +Derive the cell file from published module trees without rebuilding images + +``` +codefly deploy gitops cell [flags] +``` + +Flags: + +``` + --env string Environment to promote (default "local") +``` + ## `codefly deploy gitops observe` Verify Argo CD reconciled the reviewed Git revision and store evidence @@ -2382,6 +2397,11 @@ not. Both are reported. Values are never printed: the plan names keys and sources only. +--supplied names a JSON object of secret key to value for the required ones: a +supplied value is written where nothing holds the key, never over a stored value +and never in place of a declared generator. An empty value is accepted only for +a key service-secrets.may-be-empty declares. + --dry-run prints the plan and writes nothing. --metadata-only additionally never reads a stored value: it knows which remote keys exist, not what they hold. @@ -2409,13 +2429,17 @@ codefly deploy secrets [flags] Flags: ``` - --allow-missing Write what can be resolved even when some properties must still be supplied - --dry-run Print the plan and write nothing - --env string Environment whose rendered ExternalSecrets to seed (default "local") - --metadata-only With --dry-run: never read a stored value, only which remote keys exist - --module strings Plan only these modules' remote keys (comma-separated or repeated) - --rotate strings Replace a generated key's stored value and every stored value derived from it: //:, :, or the CODEFLY__ key (repeatable) - -y, --yes Write without an interactive confirmation + --allow-missing Write what can be resolved even when some properties must still be supplied + --dry-run Print the plan and write nothing + --env string Environment whose rendered ExternalSecrets to seed (default "local") + --export-input string Export declarative planner inputs for execution by an infrastructure-owned publisher; reads no stored values + --input string Run exported planner inputs under an infrastructure-bound Azure workload identity + --metadata-only With --dry-run: never read a stored value, only which remote keys exist + --mint strings Acknowledge first creation of these signing secret keys (comma-separated or repeated); never rotate existing keys + --module strings Plan only these modules' remote keys (comma-separated or repeated) + --rotate strings Replace a generated key's stored value and every stored value derived from it: //:, :, or the CODEFLY__ key (repeatable) + --supplied string JSON object of operator values by secret key, for keys nothing stores or generates; never printed + -y, --yes Write without an interactive confirmation ``` ## `codefly deploy service` @@ -2702,10 +2726,12 @@ codefly environment import --coordinate-contract Flags: ``` - --coordinate-contract string Path to a codefly/coordinate/v1 descriptor, or - for stdin - --dry-run Print the unified diff of workspace.codefly.yaml and write nothing - --namespace string Assert the namespace declared by the producer - --product-intent string Authored product decisions: environments..exclude-services and service-secrets generate / may-be-empty + --configuration-profiles strings Product configuration profile chain when absent from the coordinate declaration + --configuration-source string Authored product profile inherited by the imported environment + --coordinate-contract string Path to a codefly/coordinate/v1 descriptor, or - for stdin + --dry-run Print the unified diff of workspace.codefly.yaml and write nothing + --namespace string Assert the namespace declared by the producer + --product-intent string Authored product decisions: environments..exclude-services and service-secrets generate / may-be-empty ``` ## `codefly environment show` diff --git a/docs/commands.md b/docs/commands.md index 554ae7835..e6930c76d 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2758,6 +2758,19 @@ outputs, preserving the environment's separately generated runnable bindings. Run `codefly generate runnable-bindings --env blue` after import so addresses are resolved for blue rather than copied from a sibling. +For a placement-only coordinate, the product may select its own source at import: + +```bash +codefly environment import azure --coordinate-contract coordinate.json \ + --configuration-source hosted-azure --configuration-profiles azure,local +``` + +These options use the same importer and profile inheritance. A source conflicting +with the coordinate is refused; an explicit profile chain cannot replace a chain +or single profile declared by the coordinate. Without a profile chain, the target +defaults to the environment name. Later placement-only imports preserve the +product selection and regenerate its inherited profile. + The cell inventory also carries `source.path` when publication uses a root other than `deployments`. Consumers must use that root to discover the published module trees. The local render directory remains a qualification tree; `publish` writes diff --git a/docs/deployment-secret-safety.md b/docs/deployment-secret-safety.md index 61419a78b..e25519077 100644 --- a/docs/deployment-secret-safety.md +++ b/docs/deployment-secret-safety.md @@ -13,8 +13,65 @@ Backend stdout and stderr are withheld on failure, including debug formatting of backend errors. Backend diagnostics can contain payloads or HTTP response bodies; forwarding them is unsafe. The error names the command that failed and the planner adds the affected remote key. JSON decoding failures likewise omit the -backend content. Values sent to the store travel on stdin, never in process -arguments. Rendering stores references, not secret payloads. +backend content. GCP values travel on stdin and Azure values in the SDK's HTTPS +request body, never in process arguments. Rendering stores references, not secret +payloads. + +## Azure Key Vault + +The existing `azurekv` SecretStore or ClusterSecretStore selects the vault. +Publication supports Azure public-cloud WorkloadIdentity stores. The tenant comes +from the store's `tenantId` or its referenced service account's +`azure.workload.identity/tenant-id` annotation, exactly one source. A namespaced +store cannot reference a service account in another namespace. + +The Microsoft Key Vault SDK defaults to the operator's Azure CLI login, pinned to +that tenant. An authorized in-cluster publisher can explicitly select +`CODEFLY_AZURE_CREDENTIAL=workload`; the SDK then uses its projected +`AZURE_FEDERATED_TOKEN_FILE`, `AZURE_CLIENT_ID` and `AZURE_TENANT_ID`. The tenant must +match the store's declaration. Neither mode falls back to another credential, and +neither borrows the service account ESO uses to read secrets. The publisher must +already have its own write grant; this option creates no identity, grant or Job. +A mismatched vault tenant is refused by the SDK's authentication +challenge validation. Metadata-only planning lists active/deleted secret metadata +and versions; it never fetches a secret value. Disabled, expired, not-yet-valid, +managed and recoverable deleted secrets are refused, not treated as absent. +SDK errors omit response bodies and authentication diagnostics. + +An infrastructure-owned publisher can use `deploy secrets --export-input ` +to export the render's requirements and generator declarations, without reading +stored values or including credentials. `deploy secrets --input --yes` +runs the same planner inside the writer Job. Infrastructure binds the live +ClusterSecretStore name, vault URL and tenant through the Job environment; +the input cannot choose them. Execution requires workload identity and refuses +mixed stores, unknown input fields, explicit signing-key minting and partial +publication. `--dry-run` reports required values before any store write. + +Build or reuse: searched the existing CLI planner and Azure SDK adapter, infra-base's +vault seeder and runtime-credential bootstrap, and the handbook service inventory. +Reuse the planner's preservation and concurrency checks under the existing seeder +identity. The old seeder's document PUT loop cannot safely replace this planner; +no new secret backend, credential exchange or operator write grant is introduced. + +## Retained credentials and concurrent writers + +Existing keys with no readable version fail planning. First creation of an +`ed25519-pkcs8` signing key requires `--mint CODEFLY__…` naming its exact stored +configuration key; `--yes` alone is insufficient. An existing document missing +that signing property must have its original material restored. `--mint` does +not permit reminting it. Renaming an entire remote key still needs human review: +the current render cannot identify material filed under an old, unreferenced key. + +Apply checks all read sources against the plan before writing and checks each +destination again immediately before writing. Changed metadata or payloads abort +with the keys already written returned to the caller. Rebuild the plan after a +failure; existing values are retained on the retry. + +These are optimistic checks, not an atomic compare-and-set or an all-stores +transaction. GCP version creation and Azure Set Secret do not offer a conditional +version-creation operation here. Serialize writers to an environment; another +writer acting between the final check and write can still race. Runtime ESO +projection and grants require separate live qualification. ## Existing environments @@ -35,6 +92,15 @@ Configuration aliases must agree across every referenced property, including properties in one document. Aliases use one generated value when none exists, or reuse the consistent value already held anywhere in that alias group. +Required values the operator holds are passed with `--supplied `, a JSON +object of secret key to value, in either mode. The planner writes a supplied +value only where no remote key holds that configuration value. It refuses a +supplied value that differs from a stored one, one for a key a generator +declares, one for a key no rendered property reads, and an empty value for a +key the environment does not declare may-be-empty. Values never appear in the +plan, the output or an error. The in-cluster publisher reads the file from the +same transient Secret as its input. + An empty required property is an error. Missing values with no declared source are listed as required. Apply validates those requirements before confirmation or no-op success. `--dry-run` can report an incomplete plan without writing; diff --git a/docs/module-manifests.md b/docs/module-manifests.md index 752035ef3..9ec41a360 100644 --- a/docs/module-manifests.md +++ b/docs/module-manifests.md @@ -92,7 +92,10 @@ digest and media type, verbatim. A deployed render of a release declaring none builds the bundle from the release's own source, as it builds the release's images: `npm ci && npm run build` in the remote's npm project (`services/fe-remote`, else the component root), inside a digest-pinned Node -image, on the committed tree (HEAD) of the repository the component is +image, using the same environment-selected, host-networked BuildKit builder and +registry mirrors as service images. This reuses the existing builder provisioner; +the default Docker bridge can otherwise lose the operator's DNS path during npm +installation. The build runs on the committed tree (HEAD) of the repository the component is released from: nothing untracked or ignored reaches the builder (nor do uncommitted edits), and the module cache is never written. A component in no repository builds from its own directory alone. It archives `services/fe-remote/dist` in core's @@ -530,6 +533,35 @@ KMS is selected when `CODEFLY_RELEASE_KMS_KEY` is set and no Transit variable is, and a publish that sets both is refused naming each, never one silently winning. +Azure Key Vault uses that same release-signing interface. Set +`CODEFLY_RELEASE_AZURE_KEY` to an exact version URL +(`https://.vault.azure.net/keys//<32-digit-version>` or +`https://.managedhsm.azure.net/keys//<32-digit-version>`) and +`CODEFLY_RELEASE_AZURE_TENANT` to the tenant UUID. The operator's Azure CLI +credential is restricted to that tenant; no fallback credential chain is used. +An authorized release publisher can instead explicitly select +`CODEFLY_AZURE_CREDENTIAL=workload`, with the projected `AZURE_FEDERATED_TOKEN_FILE`, +`AZURE_CLIENT_ID` and matching `AZURE_TENANT_ID`. This selects the publisher's own +workload identity; it neither grants signing access nor approves a host trust entry. +This adapter supports public-cloud Key Vault and Managed HSM endpoints and enabled, currently +valid EC or EC-HSM P-256 keys permitting `sign`. The operator needs public-key +read and signing permissions. Private key material remains in the selected custody. +The existing Azure SDK handles each endpoint's authentication challenge and resource audience. + +For Azure, `host.release.key_id` defaults to the **entire version URL**, so different +vaults cannot share a key ID accidentally. Core still owns the envelope and +public-key pin. A platform that declares logical release generations may set +`CODEFLY_RELEASE_AZURE_KEY_ID=delivery-release-v1`; this must exactly match the +physical key name in the version URL and have the `-v` shape. +The complete Azure version URL remains mandatory and pins every read/sign +operation. The host's reviewed public-key pin still determines which key is trusted; +the logical ID is not a substitute for that pin. The adapter verifies each returned ES256 signature before +converting Azure's fixed-width result to Core's ASN.1 format. Requests have a +30-second timeout, no automatic retries, and no redirects; errors withhold +backend response bodies. Configuring Azure together with Cloud KMS or Vault +Transit is refused. This is release signing only: runtime Host encryption, +HMAC and Ed25519 key operations require their own supported backend. + Developer custody is one Ed25519 key per machine, created or loaded by `codefly delivery key` (ID `dev-` plus 16 lowercase hex characters of the SHA-256 of its public key; the OS keychain where available, otherwise a 0600 diff --git a/go.mod b/go.mod index 7e74a23d1..fd1ce0cd1 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,10 @@ go 1.27.3 require ( connectrpc.com/connect v1.21.1 connectrpc.com/cors v0.1.0 + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 + github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 + github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0 + github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0 github.com/Masterminds/semver v1.5.0 github.com/Masterminds/semver/v3 v3.5.0 github.com/Masterminds/sprig/v3 v3.3.0 @@ -67,7 +71,10 @@ require ( code.gitea.io/sdk/gitea v0.25.1 // indirect dario.cat/mergo v1.0.2 // indirect github.com/42wim/httpsig v1.2.4 // indirect + github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 // indirect + github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect + github.com/AzureAD/microsoft-authentication-library-for-go v1.7.0 // indirect github.com/Masterminds/goutils v1.1.1 // indirect github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29 // indirect github.com/ProtonMail/go-crypto v1.5.2 // indirect @@ -140,6 +147,7 @@ require ( github.com/go-openapi/swag/typeutils v0.29.2 // indirect github.com/go-openapi/swag/yamlutils v0.29.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect + github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect github.com/google/gnostic-models v0.7.1 // indirect github.com/google/go-github/v86 v86.0.0 // indirect @@ -156,6 +164,7 @@ require ( github.com/invopop/jsonschema v0.14.0 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/kevinburke/ssh_config v1.6.0 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.4.1 // indirect github.com/lufia/plan9stats v0.0.0-20260802145828-341c2f0c90b5 // indirect github.com/mattn/go-colorable v0.1.15 // indirect @@ -179,6 +188,7 @@ require ( github.com/openai/openai-go v1.12.0 // indirect github.com/pb33f/ordered-map/v2 v2.3.1 // indirect github.com/pjbgf/sha1cd v0.7.0 // indirect + github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pkg/errors v0.9.1 // indirect github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect github.com/power-devops/perfstat v0.0.0-20260805114148-88456608a4f6 // indirect diff --git a/go.sum b/go.sum index 5a622a262..4b6f23670 100644 --- a/go.sum +++ b/go.sum @@ -18,8 +18,26 @@ dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8= dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA= github.com/42wim/httpsig v1.2.4 h1:mI5bH0nm4xn7K18fo1K3okNDRq8CCJ0KbBYWyA6r8lU= github.com/42wim/httpsig v1.2.4/go.mod h1:yKsYfSyTBEohkPik224QPFylmzEBtda/kjyIAJjh3ps= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 h1:jHb/wfvRikGdxMXYV3QG/SzUOPYN9KEUUuC0Yd0/vC0= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1/go.mod h1:pzBXCYn05zvYIrwLgtK8Ap8QcjRg+0i76tMQdWN6wOk= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 h1:Hk5QBxZQC1jb2Fwj6mpzme37xbCDdNTxU7O9eb5+LB4= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1/go.mod h1:IYus9qsFobWIc2YVwe/WPjcnyCkPKtnHAqUYeebc8z0= +github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2 h1:yz1bePFlP5Vws5+8ez6T3HWXPmwOK7Yvq8QxDBD3SKY= +github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2/go.mod h1:Pa9ZNPuoNu/GztvBSKk9J1cDJW6vk/n0zLtV4mgd8N8= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 h1:fhqpLE3UEXi9lPaBRpQ6XuRW0nU7hgg4zlmZZa+a9q4= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0/go.mod h1:7dCRMLwisfRH3dBupKeNCioWYUZ4SS09Z14H+7i8ZoY= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0 h1:MaKvxE6D0KkjOg6Wd9M00iqP5PR0kUxCfiezes4JweM= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0/go.mod h1:i2h9fsTFKZorh8RdV2IcSUf/Qj98GlTkrTvUbX/s8as= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0 h1:aMFOzch6ZJo4Ct9hI4A9Y2fPen5YNRTPmkSBhe5m0ZQ= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0/go.mod h1:Oct8bx+g+DXKngU7i/LzFzYt44rmLdMu4uoofIpooVo= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0 h1:nCYfgcSyHZXJI8J0IWE5MsCGlb2xp9fJiXyxWgmOFg4= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0/go.mod h1:ucUjca2JtSZboY8IoUqyQyuuXvwbMBVwFOm0vdQPNhA= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c h1:udKWzYgxTojEKWjV8V+WSxDXJ4NFATAsZjh8iIbsQIg= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= +github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJTmL004Abzc5wDB5VtZG2PJk5ndYDgVacGqfirKxjM= +github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= +github.com/AzureAD/microsoft-authentication-library-for-go v1.7.0 h1:4iB+IesclUXdP0ICgAabvq2FYLXrJWKx1fJQ+GxSo3Y= +github.com/AzureAD/microsoft-authentication-library-for-go v1.7.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk= github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI= github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= @@ -250,6 +268,8 @@ github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPE github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/gofrs/flock v0.13.1 h1:jjREztyBeSKBZYAC+mgc1laB+xsgy4kYMf3FbKF2UBo= github.com/gofrs/flock v0.13.1/go.mod h1:sf4BFiHwnvgxa25DlQoDqXQnwRMEOwqxRq37P6MzzmE= +github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= +github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 h1:f+oWsMOmNPc8JmEHVZIycC7hBoQxHH9pNKQORJNozsQ= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8/go.mod h1:wcDNUvekVysuuOpQKo3191zZyTpiI6se1N1ULghS0sw= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= @@ -300,6 +320,8 @@ github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOl github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/kevinburke/ssh_config v1.6.0 h1:J1FBfmuVosPHf5GRdltRLhPJtJpTlMdKTBjRgTaQBFY= github.com/kevinburke/ssh_config v1.6.0/go.mod h1:q2RIzfka+BXARoNexmF9gkxEX7DmvbW9P4hIVx2Kg4M= +github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= +github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= @@ -307,6 +329,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= +github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lucasb-eyer/go-colorful v1.4.1 h1:1EO+WB73+EH8EVbzlrG3KLAfEypQWVHIBqlTf+2hNss= github.com/lucasb-eyer/go-colorful v1.4.1/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/lufia/plan9stats v0.0.0-20260802145828-341c2f0c90b5 h1:eveIIGn4BGM3qknO74omf6HYr30/exH+eVUTuAgwjZ0= @@ -373,6 +397,8 @@ github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdD github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pjbgf/sha1cd v0.7.0 h1:ZRNPKHj+gfkLBf0KJv/p1Hmz+7bqCT5o311YX0nq+DA= github.com/pjbgf/sha1cd v0.7.0/go.mod h1:pKR5Li+qTCo+ebqITZfwPlJGoCFCvSO00qxjbNtTUFQ= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= diff --git a/pkg/delivery/signing/azure_key_vault.go b/pkg/delivery/signing/azure_key_vault.go new file mode 100644 index 000000000..182f06af3 --- /dev/null +++ b/pkg/delivery/signing/azure_key_vault.go @@ -0,0 +1,179 @@ +package signing + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/sha256" + "encoding/asn1" + "errors" + "fmt" + "math/big" + "net/http" + "net/url" + "os" + "regexp" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys" + "github.com/codefly-dev/cli/pkg/internal/azureauth" + "github.com/google/uuid" +) + +var azureReleaseHost = regexp.MustCompile(`^[a-z][a-z0-9-]{1,22}[a-z0-9]\.(vault|managedhsm)\.azure\.net$`) +var azureReleasePath = regexp.MustCompile(`^/keys/([A-Za-z0-9-]{1,127})/([a-f0-9]{32})$`) +var azureLogicalReleaseID = regexp.MustCompile(`^[a-z0-9-]+-v[1-9][0-9]*$`) + +// AzureKeyVault signs with one explicitly versioned P-256 key. Only the public +// key leaves the vault; Core still owns delivery envelopes and admission. +type AzureKeyVault struct { + client *azkeys.Client + keyURI, name, version string + keyID string + public *ecdsa.PublicKey + publicBytes []byte +} + +// NewAzureKeyVault uses the explicitly selected tenant-bound publication credential. +func NewAzureKeyVault(lookup func(string) (string, bool)) (*AzureKeyVault, error) { + return newAzureKeyVault(lookup, nil, nil) +} + +func newAzureKeyVault(lookup func(string) (string, bool), credential azcore.TokenCredential, transport policy.Transporter) (*AzureKeyVault, error) { + if lookup == nil { + lookup = os.LookupEnv + } + keyURI, _ := lookup("CODEFLY_RELEASE_AZURE_KEY") + if keyURI == "" { + return nil, ErrNoIdentity + } + u, err := url.Parse(keyURI) + if err != nil || u.Scheme != "https" || !azureReleaseHost.MatchString(u.Host) || u.User != nil || + u.RawQuery != "" || u.ForceQuery || u.Fragment != "" || u.RawPath != "" { + return nil, errors.New("CODEFLY_RELEASE_AZURE_KEY must be an HTTPS public-cloud Azure Key Vault or Managed HSM key-version URL") + } + parts := azureReleasePath.FindStringSubmatch(u.Path) + if parts == nil { + return nil, errors.New("CODEFLY_RELEASE_AZURE_KEY requires an exact key name and 32-digit version") + } + keyID := keyURI + if declared, _ := lookup("CODEFLY_RELEASE_AZURE_KEY_ID"); declared != "" { + if !azureLogicalReleaseID.MatchString(declared) || declared != parts[1] { + return nil, errors.New("CODEFLY_RELEASE_AZURE_KEY_ID must match the versioned logical key name in CODEFLY_RELEASE_AZURE_KEY") + } + keyID = declared + } + if credential == nil { + tenant, _ := lookup("CODEFLY_RELEASE_AZURE_TENANT") + id, parseErr := uuid.Parse(tenant) + if parseErr != nil || id == uuid.Nil || id.String() != tenant { + return nil, errors.New("CODEFLY_RELEASE_AZURE_TENANT must name the declared tenant UUID") + } + credential, err = azureauth.Credential(tenant, lookup) + if err != nil { + return nil, err + } + } + if transport == nil { + transport = &http.Client{Timeout: 30 * time.Second, + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} + } + client, err := azkeys.NewClient("https://"+u.Host, credential, &azkeys.ClientOptions{ + ClientOptions: azcore.ClientOptions{Transport: transport, Retry: policy.RetryOptions{MaxRetries: -1}}, + }) + if err != nil { + return nil, errors.New("cannot initialize Azure release custody") + } + signer := &AzureKeyVault{client: client, keyURI: keyURI, name: parts[1], version: parts[2], keyID: keyID} + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + response, err := client.GetKey(ctx, signer.name, signer.version, nil) + if err != nil { + return nil, azureSigningError(err) + } + signer.public, err = azureReleasePublicKey(keyURI, response.Key, response.Attributes) + if err != nil { + return nil, err + } + signer.publicBytes, err = signer.public.Bytes() + if err != nil { + return nil, errors.New("invalid Azure release public key encoding") + } + return signer, nil +} + +func azureReleasePublicKey(keyURI string, key *azkeys.JSONWebKey, attrs *azkeys.KeyAttributes) (*ecdsa.PublicKey, error) { + now := time.Now() + if attrs == nil || attrs.Enabled == nil || !*attrs.Enabled || + (attrs.NotBefore != nil && now.Before(*attrs.NotBefore)) || (attrs.Expires != nil && !now.Before(*attrs.Expires)) { + return nil, errors.New("the Azure release key is disabled or outside its validity period") + } + if key == nil || key.KID == nil || string(*key.KID) != keyURI || key.Kty == nil || + (*key.Kty != azkeys.KeyTypeEC && *key.Kty != azkeys.KeyTypeECHSM) || key.Crv == nil || *key.Crv != azkeys.CurveNameP256 || + len(key.X) != 32 || len(key.Y) != 32 || len(key.D) != 0 || len(key.K) != 0 { + return nil, errors.New("the Azure release custody requires the requested public P-256 key version") + } + canSign := false + for _, op := range key.KeyOps { + if op != nil && *op == azkeys.KeyOperationSign { + canSign = true + } + } + if !canSign { + return nil, errors.New("the Azure release key does not permit signing") + } + encoded := make([]byte, 65) + encoded[0] = 4 // SEC 1 uncompressed point. + copy(encoded[1:33], key.X) + copy(encoded[33:], key.Y) + public, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), encoded) + if err != nil { + return nil, errors.New("invalid Azure P-256 public key") + } + return public, nil +} + +func (s *AzureKeyVault) KeyID() string { return s.keyID } +func (s *AzureKeyVault) Public() crypto.PublicKey { + // This immutable encoding was validated during construction. + public, _ := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), s.publicBytes) + return public +} + +func (s *AzureKeyVault) Sign(message []byte) ([]byte, error) { + digest := sha256.Sum256(message) + algorithm := azkeys.SignatureAlgorithmES256 + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + response, err := s.client.Sign(ctx, s.name, s.version, azkeys.SignParameters{Algorithm: &algorithm, Value: digest[:]}, nil) + if err != nil { + return nil, azureSigningError(err) + } + if response.KID == nil || string(*response.KID) != s.keyURI || len(response.Result) != 64 { + return nil, errors.New("invalid Azure signature or a different key version") + } + // Azure ES256 returns fixed-width R || S; Core verifies ASN.1 DER. + r, scalarS := new(big.Int).SetBytes(response.Result[:32]), new(big.Int).SetBytes(response.Result[32:]) + if !ecdsa.Verify(s.public, digest[:], r, scalarS) { + return nil, ErrSignature + } + return asn1.Marshal(struct{ R, S *big.Int }{r, scalarS}) +} + +func azureSigningError(err error) error { + var response *azcore.ResponseError + if errors.As(err, &response) { + return fmt.Errorf("the Azure release custody request failed (HTTP %d; backend output withheld)", response.StatusCode) + } + return errors.New("the Azure release custody authentication or transport failed (backend output withheld)") +} + +func azureCustodyConfigured(lookup func(string) (string, bool)) bool { + key, _ := lookup("CODEFLY_RELEASE_AZURE_KEY") + tenant, _ := lookup("CODEFLY_RELEASE_AZURE_TENANT") + logicalID, _ := lookup("CODEFLY_RELEASE_AZURE_KEY_ID") + return key != "" || tenant != "" || logicalID != "" +} diff --git a/pkg/delivery/signing/azure_key_vault_test.go b/pkg/delivery/signing/azure_key_vault_test.go new file mode 100644 index 000000000..8fcd8c027 --- /dev/null +++ b/pkg/delivery/signing/azure_key_vault_test.go @@ -0,0 +1,257 @@ +package signing + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/codefly-dev/core/solutionhost" + "github.com/stretchr/testify/require" +) + +const azureTestKey = "https://release.vault.azure.net/keys/delivery/0123456789abcdef0123456789abcdef" + +type azureReleaseCredential struct{ resource string } + +func (credential azureReleaseCredential) GetToken(_ context.Context, options policy.TokenRequestOptions) (azcore.AccessToken, error) { + resource := credential.resource + if resource == "" { + resource = "https://vault.azure.net" + } + if len(options.Scopes) != 1 || options.Scopes[0] != resource+"/.default" { + return azcore.AccessToken{}, fmt.Errorf("unexpected scope") + } + return azcore.AccessToken{Token: "private-release-test-token", ExpiresOn: time.Now().Add(time.Hour)}, nil +} + +type azureReleaseTransport func(*http.Request) (*http.Response, error) + +func (f azureReleaseTransport) Do(r *http.Request) (*http.Response, error) { return f(r) } + +func azureReleaseLookup(values map[string]string) func(string) (string, bool) { + return func(key string) (string, bool) { value, ok := values[key]; return value, ok } +} + +func azureReleaseFixture(t *testing.T, mutate func(string, map[string]any), failSign bool) (*AzureKeyVault, error, *int) { + t.Helper() + return azureReleaseFixtureForKey(t, azureTestKey, "https://vault.azure.net", "EC", mutate, failSign) +} + +func azureReleaseFixtureForKey(t *testing.T, keyURI, resource, keyType string, mutate func(string, map[string]any), failSign bool, logicalID ...string) (*AzureKeyVault, error, *int) { + t.Helper() + requested, err := url.Parse(keyURI) + require.NoError(t, err) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + calls := 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") == "" { + w.Header().Set("WWW-Authenticate", `Bearer authorization="https://login.microsoftonline.com/tenant", resource="`+resource+`"`) + w.WriteHeader(http.StatusUnauthorized) + return + } + require.Equal(t, "Bearer private-release-test-token", r.Header.Get("Authorization")) + calls++ + w.Header().Set("Content-Type", "application/json") + var response map[string]any + switch r.Method { + case http.MethodGet: + require.Equal(t, requested.Path, r.URL.Path) + response = map[string]any{ + "key": map[string]any{"kid": keyURI, "kty": keyType, "crv": "P-256", "key_ops": []string{"sign", "verify"}, + "x": base64.RawURLEncoding.EncodeToString(key.X.FillBytes(make([]byte, 32))), + "y": base64.RawURLEncoding.EncodeToString(key.Y.FillBytes(make([]byte, 32)))}, + "attributes": map[string]any{"enabled": true}, + } + case http.MethodPost: + require.Equal(t, requested.Path+"/sign", r.URL.Path) + if failSign { + w.WriteHeader(http.StatusForbidden) + fmt.Fprint(w, `{"error":{"message":"private-release-test-token"}}`) + return + } + var request struct { + Algorithm string `json:"alg"` + Value string `json:"value"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + require.Equal(t, "ES256", request.Algorithm) + digest, err := base64.RawURLEncoding.DecodeString(request.Value) + require.NoError(t, err) + require.Len(t, digest, 32) + r, s, err := ecdsa.Sign(rand.Reader, key, digest) + require.NoError(t, err) + raw := append(r.FillBytes(make([]byte, 32)), s.FillBytes(make([]byte, 32))...) + response = map[string]any{"kid": keyURI, "value": base64.RawURLEncoding.EncodeToString(raw)} + default: + t.Errorf("unexpected operation %s", r.Method) + } + if mutate != nil { + mutate(r.Method, response) + } + require.NoError(t, json.NewEncoder(w).Encode(response)) + })) + t.Cleanup(server.Close) + u, err := url.Parse(server.URL) + require.NoError(t, err) + client := server.Client() + client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } + transport := azureReleaseTransport(func(r *http.Request) (*http.Response, error) { + local := r.Clone(r.Context()) + local.Host, local.URL.Host = r.URL.Host, u.Host + response, err := client.Do(local) + if response != nil { + response.Request = r + } + return response, err + }) + values := map[string]string{"CODEFLY_RELEASE_AZURE_KEY": keyURI} + if len(logicalID) > 0 { + values["CODEFLY_RELEASE_AZURE_KEY_ID"] = logicalID[0] + } + signer, err := newAzureKeyVault(azureReleaseLookup(values), azureReleaseCredential{resource: resource}, transport) + return signer, err, &calls +} + +func TestAzureReleaseLogicalIDKeepsExactRemoteVersionAndCoreAdmission(t *testing.T) { + keyURI := strings.Replace(azureTestKey, "/delivery/", "/delivery-release-v1/", 1) + signer, err, calls := azureReleaseFixtureForKey(t, keyURI, "https://vault.azure.net", "EC", nil, false, "delivery-release-v1") + require.NoError(t, err) + check, err := KeyCheck(signer, solutionhost.KeyKindRelease, "acme", "delivery-release-v1") + require.NoError(t, err) + payload := testPayload(t) + envelope, err := SignEnvelope(payload, signer) + require.NoError(t, err) + require.NoError(t, check(context.Background(), envelope, payload)) + require.Equal(t, 2, *calls) + for _, mismatched := range []string{"delivery-release-v2", "another-key-v1", "delivery-release"} { + _, err, calls = azureReleaseFixtureForKey(t, keyURI, "https://vault.azure.net", "EC", nil, false, mismatched) + require.Error(t, err) + require.Zero(t, *calls) + } +} + +func TestAzureManagedHSMReleaseSignsTheCoreDeliveryEnvelope(t *testing.T) { + keyURI := strings.Replace(azureTestKey, ".vault.", ".managedhsm.", 1) + signer, err, calls := azureReleaseFixtureForKey(t, keyURI, "https://managedhsm.azure.net", "EC-HSM", nil, false) + require.NoError(t, err) + check, err := KeyCheck(signer, solutionhost.KeyKindRelease, "acme", keyURI) + require.NoError(t, err) + payload := testPayload(t) + envelope, err := SignEnvelope(payload, signer) + require.NoError(t, err) + require.NoError(t, check(context.Background(), envelope, payload)) + require.Equal(t, 2, *calls) +} + +func TestAzureManagedHSMRefusesAnotherResourceChallenge(t *testing.T) { + keyURI := strings.Replace(azureTestKey, ".vault.", ".managedhsm.", 1) + _, err, calls := azureReleaseFixtureForKey(t, keyURI, "https://vault.azure.net", "EC-HSM", nil, false) + require.Error(t, err) + require.Zero(t, *calls) +} + +func TestAzureReleaseSignsTheCoreDeliveryEnvelope(t *testing.T) { + signer, err, calls := azureReleaseFixture(t, nil, false) + require.NoError(t, err) + check, err := KeyCheck(signer, solutionhost.KeyKindRelease, "acme", azureTestKey) + require.NoError(t, err) + payload := testPayload(t) + envelope, err := SignEnvelope(payload, signer) + require.NoError(t, err) + require.NoError(t, check(context.Background(), envelope, payload)) + require.Equal(t, 2, *calls) + _, err = KeyCheck(signer, solutionhost.KeyKindRelease, "acme", "another-version") + require.Error(t, err) + wrongDomain, err := KeyCheck(signer, solutionhost.KeyKindRelease, "other", azureTestKey) + require.NoError(t, err) + require.Error(t, wrongDomain(context.Background(), envelope, payload)) + // A caller cannot mutate the key the custody later verifies against. + signer.Public().(*ecdsa.PublicKey).X.SetInt64(0) + _, err = signer.Sign([]byte("next document")) + require.NoError(t, err) +} + +func TestAzureReleaseRefusesInvalidKeys(t *testing.T) { + for _, name := range []string{"version", "curve", "disabled", "expired", "not-yet-valid", "operations", "point", "private"} { + t.Run(name, func(t *testing.T) { + _, err, _ := azureReleaseFixture(t, func(_ string, body map[string]any) { + key, attrs := body["key"].(map[string]any), body["attributes"].(map[string]any) + switch name { + case "version": + key["kid"] = azureTestKey + "other" + case "curve": + key["crv"] = "P-384" + case "disabled": + attrs["enabled"] = false + case "expired": + attrs["exp"] = time.Now().Add(-time.Hour).Unix() + case "not-yet-valid": + attrs["nbf"] = time.Now().Add(time.Hour).Unix() + case "operations": + key["key_ops"] = []string{"verify"} + case "point": + key["x"] = base64.RawURLEncoding.EncodeToString(make([]byte, 32)) + case "private": + key["d"] = "AQ" + } + }, false) + require.Error(t, err) + }) + } +} + +func TestAzureReleaseRefusesWrongSignaturesAndRedactsFailures(t *testing.T) { + for _, mode := range []string{"version", "invalid", "short", "denied"} { + t.Run(mode, func(t *testing.T) { + signer, err, calls := azureReleaseFixture(t, func(method string, body map[string]any) { + if method != http.MethodPost { + return + } + switch mode { + case "version": + body["kid"] = azureTestKey + "other" + case "invalid": + body["value"] = base64.RawURLEncoding.EncodeToString(make([]byte, 64)) + case "short": + body["value"] = "AQ" + } + }, mode == "denied") + require.NoError(t, err) + _, err = signer.Sign([]byte("document")) + require.Error(t, err) + require.NotContains(t, err.Error(), "private-release-test-token") + require.Equal(t, 2, *calls, "failed signing must not retry") + }) + } +} + +func TestAzureReleaseRefusesUnsafeConfigurationBeforeNetwork(t *testing.T) { + for _, key := range []string{ + strings.Replace(azureTestKey, "https:", "http:", 1), strings.Replace(azureTestKey, "vault.azure.net", "evil.example", 1), + azureTestKey + "?query=1", azureTestKey + "?", azureTestKey + "#fragment", azureTestKey + "/extra", + "https://release.vault.azure.net/keys/delivery", strings.Replace(azureTestKey, "/keys/", "/%6beys/", 1), + strings.Replace(azureTestKey, "vault.azure.net", "managedhsm.azure.net.evil.example", 1), + } { + _, err := NewAzureKeyVault(azureReleaseLookup(map[string]string{"CODEFLY_RELEASE_AZURE_KEY": key})) + require.Error(t, err) + } + _, err := NewAzureKeyVault(azureReleaseLookup(map[string]string{"CODEFLY_RELEASE_AZURE_KEY": azureTestKey})) + require.ErrorContains(t, err, "TENANT") + for _, other := range []string{"CODEFLY_RELEASE_KMS_KEY", "VAULT_TOKEN"} { + _, err := newReleaseSigner(azureReleaseLookup(map[string]string{"CODEFLY_RELEASE_AZURE_KEY": azureTestKey, other: "set"})) + require.ErrorContains(t, err, "two release custodies") + } +} diff --git a/pkg/delivery/signing/cloud_kms.go b/pkg/delivery/signing/cloud_kms.go index 6e92ab6d8..80675eced 100644 --- a/pkg/delivery/signing/cloud_kms.go +++ b/pkg/delivery/signing/cloud_kms.go @@ -44,6 +44,7 @@ const kmsScope = "https://www.googleapis.com/auth/cloudkms" const kmsVersionShape = "projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/" var kmsEnumName = regexp.MustCompile(`^[A-Z0-9_]{1,64}$`) +var googleCredentialStatus = regexp.MustCompile(`^oauth2/google(?:/externalaccount)?: status code ([1-5][0-9]{2}):`) // CloudKMS is the optional adapter the handbook decision admits behind the // same custody interface as Vault Transit: public metadata and an @@ -209,7 +210,14 @@ func (s *CloudKMS) request(method, operation string, body []byte, result any) er if err != nil { var exchange *oauth2.RetrieveError if errors.As(err, &exchange) && exchange.Response != nil { - return fmt.Errorf("the Google credential was not exchanged for a Cloud KMS token (HTTP %d)", exchange.Response.StatusCode) + return kmsCredentialRefusal(strconv.Itoa(exchange.Response.StatusCode), string(exchange.Body)) + } + // Google's impersonation/STS token sources return an untyped error + // containing the response body. Retain only its known status prefix. + for cause := err; cause != nil; cause = errors.Unwrap(cause) { + if status := googleCredentialStatus.FindStringSubmatch(cause.Error()); status != nil { + return kmsCredentialRefusal(status[1], strings.TrimPrefix(cause.Error(), status[0])) + } } return fmt.Errorf("the Cloud KMS %s request failed", stage) } @@ -223,6 +231,28 @@ func (s *CloudKMS) request(method, operation string, body []byte, result any) er return nil } +// Only named Google error reasons may leave a credential response; messages and +// metadata can echo credentials or request bodies. +func kmsCredentialRefusal(status, body string) error { + message := fmt.Sprintf("the Google credential was not exchanged for a Cloud KMS token (HTTP %s)", status) + var response struct { + Error struct { + Details []struct { + Reason string `json:"reason"` + } `json:"details"` + } `json:"error"` + } + if len(body) <= 65536 && json.Unmarshal([]byte(body), &response) == nil { + for _, detail := range response.Error.Details { + switch detail.Reason { + case "ACCESS_TOKEN_SCOPE_INSUFFICIENT", "IAM_PERMISSION_DENIED", "SERVICE_DISABLED", "SECURITY_POLICY_VIOLATED": + return fmt.Errorf("%s: %s", message, detail.Reason) + } + } + } + return errors.New(message) +} + // kmsHost admits only Google's API hosts, where the ambient credential's // bearer token belongs, and loopback for a fake. func kmsHost(address string) bool { diff --git a/pkg/delivery/signing/cloud_kms_test.go b/pkg/delivery/signing/cloud_kms_test.go index d1f03d6df..cab905ec1 100644 --- a/pkg/delivery/signing/cloud_kms_test.go +++ b/pkg/delivery/signing/cloud_kms_test.go @@ -11,6 +11,7 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" + "errors" "fmt" "io" "net/http" @@ -257,6 +258,34 @@ func TestCloudKMSRefusalsNeverExposeToken(t *testing.T) { } } +type failedKMSCredential struct{ err error } + +func (c failedKMSCredential) Token() (*oauth2.Token, error) { return nil, c.err } + +func TestCloudKMSCredentialRefusalsKeepStatusWithoutResponseBody(t *testing.T) { + for _, prefix := range []string{"oauth2/google", "oauth2/google/externalaccount"} { + credential := failedKMSCredential{errors.New(prefix + ": status code 403: private-test-token")} + _, err := NewCloudKMS(kmsLookup("http://localhost"), credential) + if err == nil || !strings.Contains(err.Error(), "HTTP 403") || strings.Contains(err.Error(), "private-test-token") { + t.Fatalf("unsafe or unhelpful credential refusal: %v", err) + } + } +} + +func TestCloudKMSCredentialReasonsAreAllowlisted(t *testing.T) { + for _, reason := range []string{"ACCESS_TOKEN_SCOPE_INSUFFICIENT", "IAM_PERMISSION_DENIED", "private-test-token"} { + body := `{"error":{"message":"private-test-token","details":[{"reason":"` + reason + `"}]}}` + credential := failedKMSCredential{errors.New("oauth2/google: status code 403: " + body)} + _, err := NewCloudKMS(kmsLookup("http://localhost"), credential) + if err == nil || strings.Contains(err.Error(), "private-test-token") || !strings.Contains(err.Error(), "HTTP 403") { + t.Fatalf("unsafe or unhelpful refusal: %v", err) + } + if reason != "private-test-token" && !strings.Contains(err.Error(), reason) { + t.Fatalf("known reason omitted: %v", err) + } + } +} + func TestCloudKMSRejectsUnsafeConfiguration(t *testing.T) { for _, endpoint := range []string{"http://example.test", "http://192.0.2.1", "ftp://localhost", "https://private-test-token@example.test", "https://example.test?token=private-test-token", // The bearer token belongs to Google's hosts only. diff --git a/pkg/delivery/signing/signing.go b/pkg/delivery/signing/signing.go index 64b76ad8f..e18f0f6f5 100644 --- a/pkg/delivery/signing/signing.go +++ b/pkg/delivery/signing/signing.go @@ -15,7 +15,7 @@ import ( // Signer is core's custody boundary. Core owns envelope construction and verification. type Signer = solutionhost.Signer -var ErrNoIdentity = errors.New("no delivery signing key: release publishing requires VAULT_ADDR, VAULT_TOKEN and CODEFLY_RELEASE_TRANSIT_KEY (Vault Transit custody) or CODEFLY_RELEASE_KMS_KEY (Cloud KMS custody)") +var ErrNoIdentity = errors.New("no delivery signing key: release publishing requires VAULT_ADDR, VAULT_TOKEN and CODEFLY_RELEASE_TRANSIT_KEY (Vault Transit custody), CODEFLY_RELEASE_KMS_KEY (Cloud KMS custody), or CODEFLY_RELEASE_AZURE_KEY and CODEFLY_RELEASE_AZURE_TENANT (Azure Key Vault custody)") var ErrSignature = errors.New("delivery document signature verification failed") // Unavailable records why this process cannot sign. diff --git a/pkg/delivery/signing/vault_transit.go b/pkg/delivery/signing/vault_transit.go index f5ee31eea..56e812159 100644 --- a/pkg/delivery/signing/vault_transit.go +++ b/pkg/delivery/signing/vault_transit.go @@ -48,10 +48,8 @@ func FromEnvironment(lookup func(string) (string, bool)) Signer { return signer } -// newReleaseSigner selects the one custody the environment configures: Cloud -// KMS when CODEFLY_RELEASE_KMS_KEY is set and no Transit variable is, Vault -// Transit otherwise. Both configured is refused, naming each, rather than -// one silently winning. +// newReleaseSigner selects exactly one configured release custody: Azure Key +// Vault, Google Cloud KMS, or Vault Transit. Conflicting selections are refused. func newReleaseSigner(lookup func(string) (string, bool)) (Signer, error) { if lookup == nil { lookup = os.LookupEnv @@ -62,6 +60,13 @@ func newReleaseSigner(lookup func(string) (string, bool)) (Signer, error) { transit = append(transit, name) } } + if azureCustodyConfigured(lookup) { + kms, _ := lookup("CODEFLY_RELEASE_KMS_KEY") + if kms != "" || len(transit) > 0 { + return nil, errors.New("two release custodies are configured: Azure Key Vault and Cloud KMS or Vault Transit; unset one") + } + return NewAzureKeyVault(lookup) + } if kms, _ := lookup("CODEFLY_RELEASE_KMS_KEY"); kms != "" { if len(transit) > 0 { return nil, fmt.Errorf("two release custodies are configured: Cloud KMS (CODEFLY_RELEASE_KMS_KEY) and Vault Transit (%s); unset one", strings.Join(transit, ", ")) diff --git a/pkg/deploysecrets/azure.go b/pkg/deploysecrets/azure.go new file mode 100644 index 000000000..722b6414f --- /dev/null +++ b/pkg/deploysecrets/azure.go @@ -0,0 +1,265 @@ +package deploysecrets + +import ( + "context" + "crypto/sha256" + "encoding/json" + "errors" + "fmt" + "net/url" + "regexp" + "slices" + "strings" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/cli/pkg/internal/azureauth" +) + +// AzureKeyVault uses the selected publisher identity, restricted to the ESO +// store's tenant. It never borrows ESO's credential or falls back to another +// credential source. The SDK owns authentication and paging. +type AzureKeyVault struct { + vaultURL string + client *azsecrets.Client +} + +var azureSecretName = regexp.MustCompile(`^[A-Za-z0-9-]{1,127}$`) +var azureVaultHost = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]{1,22}[a-zA-Z0-9]\.vault\.azure\.net$`) + +// AzurePublisherStore reuses the live-store adapter inside an infrastructure-owned +// publisher Job. Infrastructure supplies the destination, never the input document. +func AzurePublisherStore(ctx context.Context, vaultURL, tenantID string) (Store, error) { + raw, err := json.Marshal(map[string]string{"vaultUrl": vaultURL, "tenantId": tenantID, "authType": "WorkloadIdentity"}) + if err != nil { + return nil, errors.New("invalid publisher store binding") + } + return resolveAzureStore(ctx, nil, ClusterTarget{}, environments.EnvironmentSecretStoreReference{}, "", raw) +} + +func resolveAzureStore(ctx context.Context, run Runner, target ClusterTarget, ref environments.EnvironmentSecretStoreReference, namespace string, raw json.RawMessage) (Store, error) { + var provider struct { + VaultURL string `json:"vaultUrl"` + TenantID string `json:"tenantId"` + AuthType string `json:"authType"` + EnvironmentType string `json:"environmentType"` + ServiceAccountRef struct{ Name, Namespace string } `json:"serviceAccountRef"` + } + if json.Unmarshal(raw, &provider) != nil { + return nil, errors.New("invalid azurekv provider configuration (backend output withheld)") + } + u, parseErr := url.Parse(provider.VaultURL) + if parseErr != nil || u.Scheme != "https" || !azureVaultHost.MatchString(u.Host) || u.User != nil || + (u.Path != "" && u.Path != "/") || u.RawQuery != "" || u.Fragment != "" { + return nil, errors.New("azurekv requires an HTTPS Azure public-cloud vault URL") + } + if provider.AuthType != "WorkloadIdentity" || (provider.EnvironmentType != "" && provider.EnvironmentType != "PublicCloud") { + return nil, errors.New("azurekv publication currently supports PublicCloud WorkloadIdentity stores") + } + if provider.ServiceAccountRef.Name != "" { + saNamespace := provider.ServiceAccountRef.Namespace + if ref.Kind == "SecretStore" { + if saNamespace != "" && saNamespace != namespace { + return nil, errors.New("azurekv SecretStore service account must be in the store namespace") + } + saNamespace = namespace + } + if saNamespace == "" { + return nil, errors.New("azurekv ClusterSecretStore service account requires a namespace") + } + output, err := run(ctx, nil, "kubectl", "get", "serviceaccount", provider.ServiceAccountRef.Name, + "--namespace", saNamespace, "--kubeconfig", target.Kubeconfig, "--context", target.Context, "-o", "json") + if err != nil { + return nil, err + } + var account struct { + Metadata struct{ Annotations map[string]string } + } + if json.Unmarshal(output, &account) != nil { + return nil, errors.New("invalid azurekv service account metadata (backend output withheld)") + } + tenant := account.Metadata.Annotations["azure.workload.identity/tenant-id"] + if provider.TenantID != "" && tenant != "" { + return nil, errors.New("azurekv tenant is declared twice; use the same single tenant source as ESO") + } + if tenant != "" { + provider.TenantID = tenant + } + } + if provider.TenantID == "" { + return nil, errors.New("azurekv declares no tenant identity") + } + credential, err := azureauth.Credential(provider.TenantID, nil) + if err != nil { + return nil, err + } + client, err := azsecrets.NewClient(strings.TrimRight(provider.VaultURL, "/"), credential, nil) + if err != nil { + return nil, errors.New("cannot initialize Azure Key Vault client") + } + return &AzureKeyVault{vaultURL: strings.TrimRight(provider.VaultURL, "/"), client: client}, nil +} + +func (store *AzureKeyVault) Name() string { return "azurekv " + store.vaultURL } + +func (store *AzureKeyVault) Describe(ctx context.Context, key string) (Description, error) { + if !azureSecretName.MatchString(key) { + return Description{}, errors.New("invalid Azure Key Vault secret name") + } + pager := store.client.NewListSecretPropertiesPager(nil) + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + return Description{}, azureError(err) + } + if page.Value == nil { + return Description{}, errors.New("incomplete Azure secret listing") + } + for _, property := range page.Value { + if property == nil || property.ID == nil { + return Description{}, errors.New("incomplete Azure secret metadata") + } + if !strings.EqualFold(property.ID.Name(), key) { + continue + } + attributes := property.Attributes + if attributes == nil || attributes.Enabled == nil { + return Description{}, errors.New("incomplete Azure secret attributes") + } + now := time.Now() + if !*attributes.Enabled || (attributes.Expires != nil && !now.Before(*attributes.Expires)) || + (attributes.NotBefore != nil && now.Before(*attributes.NotBefore)) || (property.Managed != nil && *property.Managed) { + return Description{}, errors.New("azure secret is disabled, expired, not yet valid or externally managed; refusing to replace it") + } + // Include every version's metadata: listing base properties alone can + // miss two new versions created within the same timestamp second. + metadata, _ := json.Marshal(property) + versions := []string{string(metadata)} + vp := store.client.NewListSecretPropertiesVersionsPager(key, nil) + for vp.More() { + page, err := vp.NextPage(ctx) + if err != nil { + return Description{}, azureError(err) + } + for _, version := range page.Value { + if version == nil || version.ID == nil { + return Description{}, errors.New("incomplete Azure version metadata") + } + encoded, _ := json.Marshal(version) + versions = append(versions, string(encoded)) + } + } + if len(versions) == 1 { + return Description{}, errors.New("azure secret has no version; refusing to replace it") + } + slices.Sort(versions) + return Description{Exists: true, HasVersion: true, Version: fmt.Sprintf("%x", sha256.Sum256([]byte(strings.Join(versions, "\n"))))}, nil + } + } + return store.describeDeleted(ctx, key) +} + +func (store *AzureKeyVault) describeDeleted(ctx context.Context, key string) (Description, error) { + // Absence from the active list is insufficient: recoverable material must + // never be purged, recovered or replaced as part of credential seeding. + deleted := store.client.NewListDeletedSecretPropertiesPager(nil) + for deleted.More() { + page, err := deleted.NextPage(ctx) + if err != nil { + return Description{}, azureError(err) + } + if page.Value == nil { + return Description{}, errors.New("incomplete deleted Azure secret listing") + } + for _, property := range page.Value { + if property == nil || property.ID == nil { + return Description{}, errors.New("incomplete deleted Azure secret metadata") + } + if strings.EqualFold(property.ID.Name(), key) { + return Description{}, errors.New("azure secret is deleted and recoverable; refusing to replace it") + } + } + } + return Description{}, nil +} + +func (store *AzureKeyVault) Read(ctx context.Context, key string) (map[string]string, error) { + if !azureSecretName.MatchString(key) { + return nil, errors.New("invalid Azure Key Vault secret name") + } + result, err := store.client.GetSecret(ctx, key, "", nil) + if err != nil { + return nil, azureError(err) + } + if result.Value == nil { + return nil, errors.New("azure secret has no value") + } + return decodeDocument(key, []byte(*result.Value)) +} + +func (store *AzureKeyVault) Write(ctx context.Context, key string, document map[string]string, create bool) (string, error) { + if !azureSecretName.MatchString(key) { + return "", errors.New("invalid Azure Key Vault secret name") + } + encoded, err := json.Marshal(document) + if err != nil { + return "", errors.New("cannot encode Azure secret document") + } + value := string(encoded) + parameters := azsecrets.SetSecretParameters{Value: &value} + if !create { + current, readErr := store.client.GetSecret(ctx, key, "", nil) + if readErr != nil { + return "", azureError(readErr) + } + parameters.ContentType, parameters.Tags = current.ContentType, current.Tags + if attributes := current.Attributes; attributes != nil { + parameters.SecretAttributes = &azsecrets.SecretAttributes{ + Enabled: attributes.Enabled, Expires: attributes.Expires, NotBefore: attributes.NotBefore, + } + } + } + _, err = store.client.SetSecret(ctx, key, parameters, nil) + if err != nil { + return "", azureError(err) + } + description, err := store.Describe(ctx, key) + return description.Version, err +} + +// Never wrap SDK errors: their response bodies and auth diagnostics may contain +// credentials. This also protects %+v, %#v and JSON error formatting. +func azureError(err error) error { + if err == nil { + return nil + } + var response *azcore.ResponseError + if errors.As(err, &response) { + reason := "" + if response.StatusCode == 403 && response.RawResponse != nil { + // Inspect only the SDK's response payload for a closed diagnostic code. + // No provider text, IDs, URLs or unknown code can enter the error. + payload, readErr := runtime.Payload(response.RawResponse) + var detail struct { + Error struct { + InnerError struct { + Code string `json:"code"` + } `json:"innererror"` + } `json:"error"` + } + if readErr == nil && len(payload) <= 65536 && json.Unmarshal(payload, &detail) == nil { + switch detail.Error.InnerError.Code { + case "ForbiddenByRbac": + reason = "; data-plane role denied" + case "ForbiddenByConnection", "ForbiddenByFirewall": + reason = "; private-network access denied" + } + } + } + return fmt.Errorf("azure Key Vault request failed (HTTP %d%s; backend output withheld)", response.StatusCode, reason) + } + return errors.New("azure Key Vault authentication or transport failed (backend output withheld)") +} diff --git a/pkg/deploysecrets/azure_test.go b/pkg/deploysecrets/azure_test.go new file mode 100644 index 000000000..c48694f33 --- /dev/null +++ b/pkg/deploysecrets/azure_test.go @@ -0,0 +1,258 @@ +package deploysecrets + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/cli/pkg/gitops" + "github.com/stretchr/testify/require" +) + +type testAzureCredential struct{} + +type azureTestTransport func(*http.Request) (*http.Response, error) + +func (f azureTestTransport) Do(r *http.Request) (*http.Response, error) { return f(r) } + +func (testAzureCredential) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) { + return azcore.AccessToken{Token: "test-only", ExpiresOn: time.Now().Add(time.Hour)}, nil +} + +func azureTestStore(t *testing.T, handler http.HandlerFunc) *AzureKeyVault { + t.Helper() + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") == "" { + w.Header().Set("WWW-Authenticate", `Bearer authorization="https://login.microsoftonline.com/test", resource="https://vault.azure.net"`) + w.WriteHeader(http.StatusUnauthorized) + return + } + handler(w, r) + })) + t.Cleanup(server.Close) + endpoint, err := url.Parse(server.URL) + require.NoError(t, err) + transport := azureTestTransport(func(r *http.Request) (*http.Response, error) { + local := r.Clone(r.Context()) + local.Host, local.URL.Host = r.URL.Host, endpoint.Host + response, err := server.Client().Do(local) + if response != nil { + response.Request = r + } + return response, err + }) + client, err := azsecrets.NewClient("https://example.vault.azure.net", testAzureCredential{}, &azsecrets.ClientOptions{ + ClientOptions: azcore.ClientOptions{Transport: transport, Retry: policy.RetryOptions{MaxRetries: -1}}, + }) + require.NoError(t, err) + return &AzureKeyVault{vaultURL: server.URL, client: client} +} + +func TestAzureDescribeIsMetadataOnlyAndFollowsPages(t *testing.T) { + var paths []string + store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/secrets": + fmt.Fprintf(w, `{"value":[],"nextLink":"https://%s/page2"}`, r.Host) + case "/page2": + fmt.Fprint(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true}}]}`) + case "/secrets/KEY/versions": + fmt.Fprint(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key/v1","attributes":{"enabled":true}}]}`) + default: + t.Errorf("unexpected payload operation %s %s", r.Method, r.URL.Path) + w.WriteHeader(500) + } + }) + description, err := store.Describe(context.Background(), "KEY") + require.NoError(t, err) + require.True(t, description.Exists) + require.True(t, description.HasVersion) + require.NotEmpty(t, description.Version) + require.Equal(t, []string{"/secrets", "/page2", "/secrets/KEY/versions"}, paths) +} + +func TestAzureDescribeRefusesUnavailableMaterial(t *testing.T) { + for _, tc := range []struct { + name, active, deleted string + status int + want string + }{ + {"missing", `{"value":[]}`, `{"value":[]}`, 200, ""}, + {"disabled", `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":false}}]}`, "", 200, "disabled"}, + {"expired", `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true,"exp":1}}]}`, "", 200, "expired"}, + {"future", `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true,"nbf":4102444800}}]}`, "", 200, "not yet valid"}, + {"deleted", `{"value":[]}`, `{"value":[{"id":"https://example.vault.azure.net/secrets/key"}]}`, 200, "recoverable"}, + {"denied", `{"error":{"code":"Forbidden","message":"private-marker"}}`, "", 403, "HTTP 403"}, + {"invalid", `{"value":[{}]}`, "", 200, "incomplete"}, + {"missing-list", `{}`, "", 200, "incomplete"}, + } { + t.Run(tc.name, func(t *testing.T) { + store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.status) + if r.URL.Path == "/deletedsecrets" { + fmt.Fprint(w, tc.deleted) + } else { + fmt.Fprint(w, tc.active) + } + }) + description, err := store.Describe(context.Background(), "key") + if tc.want == "" { + require.NoError(t, err) + require.False(t, description.Exists) + return + } + require.ErrorContains(t, err, tc.want) + require.NotContains(t, fmt.Sprintf("%v %+v %#v", err, err, err), "private-marker") + }) + } +} + +func TestAzurePlannerPreservesPropertiesAndRefusesChangedPlan(t *testing.T) { + for _, changed := range []bool{false, true} { + t.Run(fmt.Sprint(changed), func(t *testing.T) { + document := map[string]string{"unrelated": "keep-this"} + version, writes := 1, 0 + store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/secrets": + fmt.Fprint(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true}}]}`) + case "/secrets/key/versions": + fmt.Fprintf(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key/v%d","attributes":{"enabled":true}}]}`, version) + case "/secrets/key", "/secrets/key/": + if r.Method == http.MethodPut { + var body struct { + Value, ContentType string + Tags map[string]string + Attributes map[string]any + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Error(err) + } + if err := json.Unmarshal([]byte(body.Value), &document); err != nil { + t.Error(err) + } + if body.Tags["owner"] != "existing-owner" || body.ContentType != "application/json" || body.Attributes["exp"] != float64(4102444800) { + t.Error("write discarded existing secret metadata") + } + writes++ + version++ + } + value, _ := json.Marshal(document) + _ = json.NewEncoder(w).Encode(map[string]any{"value": string(value), "contentType": "application/json", "tags": map[string]string{"owner": "existing-owner"}, "attributes": map[string]any{"enabled": true, "exp": 4102444800}}) + default: + t.Errorf("unexpected request %s", r.URL.Path) + w.WriteHeader(500) + } + }) + inputs := &Inputs{Rendered: gitops.RenderedEnvironment{Secrets: []gitops.RenderedServiceSecret{ + {RemoteKey: "key", Properties: filed(internalToken)}, + }}, Store: store, ReadPayloads: true, Generators: generators()} + plan, err := Build(context.Background(), inputs) + require.NoError(t, err) + if changed { + version++ + } + written, err := plan.Apply(context.Background(), store, false) + if changed { + require.ErrorContains(t, err, "changed since planning") + require.Empty(t, written) + require.Zero(t, writes) + return + } + require.NoError(t, err) + require.Len(t, written, 1) + require.Equal(t, "key", written[0].Key) + require.Equal(t, "keep-this", document["unrelated"]) + require.NotEmpty(t, document[internalToken]) + again, err := Build(context.Background(), inputs) + require.NoError(t, err) + require.Empty(t, again.Changes()) + require.Equal(t, 1, writes) + }) + } +} + +func TestResolveAzureStoreUsesESOTenantSource(t *testing.T) { + for _, tenant := range []string{"11111111-1111-1111-1111-111111111111", ""} { + runner := &scriptedRunner{answers: map[string]func() ([]byte, error){ + "kubectl get clustersecretstore cell-secrets": func() ([]byte, error) { + return []byte(`{"spec":{"provider":{"azurekv":{"vaultUrl":"https://example.vault.azure.net","authType":"WorkloadIdentity","serviceAccountRef":{"name":"eso","namespace":"external-secrets"}}}}}`), nil + }, + "kubectl get serviceaccount eso": func() ([]byte, error) { + return []byte(fmt.Sprintf(`{"metadata":{"annotations":{"azure.workload.identity/tenant-id":%q}}}`, tenant)), nil + }, + }} + store, err := ResolveStore(context.Background(), runner.run, ClusterTarget{Kubeconfig: "/kube", Context: "azure"}, + environments.EnvironmentSecretStoreReference{Name: "cell-secrets", Kind: "ClusterSecretStore"}, "app") + if tenant == "" { + require.ErrorContains(t, err, "no tenant") + continue + } + require.NoError(t, err) + require.Equal(t, "azurekv https://example.vault.azure.net", store.Name()) + require.Contains(t, runner.calls[1].args, "external-secrets") + require.Contains(t, runner.calls[1].args, "azure") + } +} + +func TestResolveAzureRefusesAmbiguousAuthority(t *testing.T) { + for _, raw := range []string{ + `{"vaultUrl":"http://example.vault.azure.net","authType":"WorkloadIdentity","tenantId":"t"}`, + `{"vaultUrl":"https://example.vault.azure.net.attacker.test","authType":"WorkloadIdentity","tenantId":"t"}`, + `{"vaultUrl":"https://example.vault.azure.net","authType":"ServicePrincipal","tenantId":"t"}`, + `{"vaultUrl":"https://example.vault.azure.net","authType":"WorkloadIdentity"}`, + `{"vaultUrl":"https://example.vault.azure.net","authType":"WorkloadIdentity","tenantId":"t","serviceAccountRef":{"name":"eso"}}`, + } { + _, err := resolveAzureStore(context.Background(), nil, ClusterTarget{}, environments.EnvironmentSecretStoreReference{Kind: "ClusterSecretStore"}, "", json.RawMessage(raw)) + require.Error(t, err) + } +} + +func TestAzureFailureNeverExposesResponseOrPayload(t *testing.T) { + store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(403) + fmt.Fprint(w, `{"error":{"code":"Forbidden","message":"private-marker"}}`) + }) + _, err := store.Write(context.Background(), "key", map[string]string{"key": "private-marker"}, true) + require.Error(t, err) + encoded, marshalErr := json.Marshal(err) + require.NoError(t, marshalErr) + for _, output := range []string{fmt.Sprintf("%v %+v %#v", err, err, err), string(encoded)} { + require.False(t, strings.Contains(output, "private-marker")) + } +} + +func TestAzureForbiddenClassifiesOnlyKnownCodes(t *testing.T) { + for code, want := range map[string]string{ + "ForbiddenByRbac": "data-plane role denied", + "ForbiddenByConnection": "private-network access denied", + "ForbiddenByFirewall": "private-network access denied", + "private-marker": "HTTP 403; backend output withheld", + } { + t.Run(code, func(t *testing.T) { + store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(403) + _ = json.NewEncoder(w).Encode(map[string]any{"error": map[string]any{"code": "Forbidden", "message": "private-marker", "innererror": map[string]any{"code": code}}}) + }) + _, err := store.Write(context.Background(), "key", map[string]string{"key": "private-marker"}, true) + require.ErrorContains(t, err, want) + require.NotContains(t, fmt.Sprintf("%v %+v %#v", err, err, err), "private-marker") + }) + } +} diff --git a/pkg/deploysecrets/plan.go b/pkg/deploysecrets/plan.go index d794b98e8..cbd232b3e 100644 --- a/pkg/deploysecrets/plan.go +++ b/pkg/deploysecrets/plan.go @@ -11,6 +11,8 @@ // unless the operator names its generated key with --rotate (rotate.go); // 2. propagate — a configuration value is one value however many services read // it, so a key another remote secret already holds is copied from there; +// 3. supply — a key the operator handed this run, by secret key, because no +// key holds it and nothing can derive it; // 3. configured — the environment's own configuration holds the value, in the // `configurations//.secret.env` that `codefly config set` // writes (see configured.go), so the operator's existing file seeds the @@ -61,6 +63,9 @@ const ( // ActionGenerate: the property is absent and generated at random, as the // environment declares. ActionGenerate Action = "generate" + // ActionSupply: the property is absent and written from the value the + // operator supplied for its secret key. + ActionSupply Action = "supply" // ActionRequire: the property is absent and nothing can produce it; the // operator supplies it. ActionRequire Action = "require" @@ -112,6 +117,7 @@ type Plan struct { Secrets []SecretPlan writes []pendingWrite + states []*remoteState // rotating makes Apply all-or-nothing: a rotation half-written is a source // and its derived values disagreeing, so a failed write restores the rest. rotating bool @@ -197,6 +203,13 @@ type Inputs struct { // empty, from service-secrets.may-be-empty. A present-but-empty property is // otherwise refused. MayBeEmpty []string + // MintKeys explicitly acknowledges first creation of lifelong signing keys. + MintKeys []string + // Supplied are operator values for keys no remote key holds and no generator + // declares, by secret key. A supplied value never replaces a stored one and + // never stands in for a declared generator. ValidateSupplied checks them + // against the whole render first. + Supplied map[string]string // Configured are the values the environment's own configuration files hold, // by the secret key a render reads each one under — ReadConfigured. A // property no remote key holds is resolved from here before a generator is @@ -247,6 +260,7 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { if err != nil { return nil, err } + plan.states = states generators, err := generatorIndex(in.Generators, in.Services) if err != nil { @@ -277,7 +291,7 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { for _, property := range state.secret.Properties { location := propertyLocation{state.secret.RemoteKey, property.Property} propertyPlan, value, err := planConfigured(state, property, states, generators, generated, in.Configured, - aliases[location], rotating, in.MayBeEmpty) + aliases[location], rotating, in.MayBeEmpty, in.MintKeys, in.Supplied) if err != nil { return nil, err } @@ -286,7 +300,7 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { Source: "derived: the stored URL's password follows rotated " + strings.Join(rewrite.from, ", ")} value = rewrite.value } - if value != "" || propertyPlan.Action == ActionEmpty { + if value != "" || propertyPlan.Action == ActionEmpty || propertyPlan.Action == ActionSupply { changes[property.Property] = value } secretPlan.Properties = append(secretPlan.Properties, propertyPlan) @@ -355,8 +369,12 @@ func readStates(ctx context.Context, in *Inputs) ([]*remoteState, error) { return } state := &remoteState{secret: secret, description: description} + if description.Exists && !description.HasVersion { + fail(fmt.Errorf("remote key %s exists without a readable version; refusing to regenerate retained material", secret.RemoteKey)) + return + } switch { - case !description.Exists || !description.HasVersion: + case !description.Exists: state.document = map[string]string{} case in.ReadPayloads: document, err := in.Store.Read(ctx, secret.RemoteKey) @@ -364,6 +382,14 @@ func readStates(ctx context.Context, in *Inputs) ([]*remoteState, error) { return } state.document = document + current, err := in.Store.Describe(ctx, secret.RemoteKey) + if fail(wrapKeyError("describe after read", secret.RemoteKey, err)) { + return + } + if current != description { + fail(fmt.Errorf("remote key %s changed while reading; rebuild the plan", secret.RemoteKey)) + return + } } // An empty stored value is refused, because an empty credential is // almost always a half-finished seed. Two things narrow that: @@ -535,13 +561,17 @@ func refuseSharedSigningKey(state *remoteState, property gitops.RenderedSecretPr // only consulted for a property nothing holds. func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, states []*remoteState, generators map[string]environments.EnvironmentSecretGenerator, generated map[string]string, - configured map[string]ConfiguredValue, keys []string, rotating map[string]bool, mayBeEmpty []string) (PropertyPlan, string, error) { + configured map[string]ConfiguredValue, keys []string, rotating map[string]bool, mayBeEmpty, mintKeys []string, supplied map[string]string) (PropertyPlan, string, error) { lookup := property lookup.Keys = keys declaredKey, generator, err := declaredGenerator(lookup, generators) if err != nil { return PropertyPlan{}, "", err } + suppliedValue, isSupplied, err := suppliedFor(lookup, supplied, declaredKey) + if err != nil { + return PropertyPlan{}, "", err + } configuredValue, hasConfigured, err := configuredFor(lookup, configured) if err != nil { return PropertyPlan{}, "", err @@ -551,27 +581,13 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, return PropertyPlan{}, "", err } } - var holders, unread []string - var holderValue string - for _, other := range states { - for _, candidate := range other.secret.Properties { - if (other != state || candidate.Property != property.Property) && !sharesConfigurationKey(lookup, candidate) { - continue - } - location := other.secret.RemoteKey + "#" + candidate.Property - if !other.known() { - unread = append(unread, location) - continue - } - value, present := other.document[candidate.Property] - if !present { - continue - } - if len(holders) > 0 && value != holderValue { - return PropertyPlan{}, "", fmt.Errorf("one configuration value cannot be two: different values in %s and %s; resolve it before planning", holders[0], location) - } - holders = append(holders, location) - holderValue = value + holders, unread, holderValue, err := configurationHolders(state, property, lookup, states) + if err != nil { + return PropertyPlan{}, "", err + } + if len(holders) > 0 { + if err = refuseSuppliedConflict(property, isSupplied, suppliedValue, holders[0], holderValue); err != nil { + return PropertyPlan{}, "", err } } // A configured value disagreeing with what the store holds is refused, on @@ -597,9 +613,21 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, return PropertyPlan{}, "", fmt.Errorf("%s holds different values in %s and %s: one configuration value cannot be two; resolve it before planning", strings.Join(property.Keys, ", "), state.secret.RemoteKey, holders[0]) } + if err = refuseSuppliedConflict(property, isSupplied, suppliedValue, state.secret.RemoteKey+"#"+property.Property, stored); err != nil { + return PropertyPlan{}, "", err + } return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionKeep, Source: "stored"}, "", nil } } + if generator.Format == environments.SecretGeneratorFormatEd25519PKCS8 && len(holders) == 0 && len(unread) == 0 { + if state.description.Exists { + return PropertyPlan{}, "", fmt.Errorf("remote key %s exists but lacks signing key %s; restore its original material, never re-mint it", state.secret.RemoteKey, declaredKey) + } + if !slices.Contains(mintKeys, declaredKey) { + return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionRequire, + Source: "first signing-key creation requires --mint " + declaredKey}, "", nil + } + } fallback, value, err := fallbackSource(lookup, generators, generated, configuredValue, hasConfigured, mayBeEmpty) if err != nil { return PropertyPlan{}, "", err @@ -617,11 +645,106 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, case len(unread) > 0: return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionUnverified, Source: fmt.Sprintf("may propagate from %s (not read); else %s", strings.Join(unread, ", "), fallback.Source)}, "", nil + case isSupplied: + return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionSupply, Source: "supplied by the operator"}, suppliedValue, nil default: return fallback, value, nil } } +// configurationHolders lists every location holding this property's +// configuration value — this key's own property and any other property read +// under one of its keys — and the one value they agree on. Unread keys are +// listed apart; two read holders that disagree are refused. +func configurationHolders(state *remoteState, property, lookup gitops.RenderedSecretProperty, states []*remoteState) (holders, unread []string, holderValue string, err error) { + for _, other := range states { + for _, candidate := range other.secret.Properties { + if (other != state || candidate.Property != property.Property) && !sharesConfigurationKey(lookup, candidate) { + continue + } + location := other.secret.RemoteKey + "#" + candidate.Property + if !other.known() { + unread = append(unread, location) + continue + } + value, present := other.document[candidate.Property] + if !present { + continue + } + if len(holders) > 0 && value != holderValue { + return nil, nil, "", fmt.Errorf("one configuration value cannot be two: different values in %s and %s; resolve it before planning", holders[0], location) + } + holders = append(holders, location) + holderValue = value + } + } + return holders, unread, holderValue, nil +} + +// suppliedFor is the operator's value for a property, looked up by every secret +// key it is read under. Two keys of one configuration value supplied twice must +// agree, as two stored copies must, and a declared generator is never overridden. +func suppliedFor(property gitops.RenderedSecretProperty, supplied map[string]string, declaredKey string) (string, bool, error) { + var value, from string + for _, key := range property.Keys { + candidate, ok := supplied[key] + if !ok { + continue + } + if from != "" && candidate != value { + return "", false, fmt.Errorf("one configuration value cannot be two: %s and %s are supplied differently", from, key) + } + value, from = candidate, key + } + if from != "" && declaredKey != "" { + return "", false, fmt.Errorf("%s is both supplied and declared by service-secrets.generate; drop the supplied value", declaredKey) + } + return value, from != "", nil +} + +// refuseSuppliedConflict refuses a supplied value that differs from one already +// held at location: a supplied value never replaces a stored one. +func refuseSuppliedConflict(property gitops.RenderedSecretProperty, isSupplied bool, supplied, location, held string) error { + if !isSupplied || supplied == held { + return nil + } + return fmt.Errorf("the value supplied for %s differs from the one %s holds; a supplied value never replaces a stored one", + strings.Join(property.Keys, ", "), location) +} + +// ValidateSupplied refuses supplied values the render cannot use: a key no +// rendered property reads (a typo would otherwise be dropped silently), a key +// that is not a configuration key, and an empty value for a key the environment +// does not declare may-be-empty. +func ValidateSupplied(rendered gitops.RenderedEnvironment, supplied map[string]string, mayBeEmpty []string) error { + read := map[string]bool{} + for _, secret := range rendered.Secrets { + for _, property := range secret.Properties { + for _, key := range property.Keys { + read[key] = true + } + } + } + var unknown, empty []string + for key, value := range supplied { + switch { + case !configurationKey(key) || !read[key]: + unknown = append(unknown, key) + case value == "" && !slices.Contains(mayBeEmpty, key): + empty = append(empty, key) + } + } + slices.Sort(unknown) + slices.Sort(empty) + if len(unknown) > 0 { + return fmt.Errorf("supplied keys no rendered property reads: %s", strings.Join(unknown, ", ")) + } + if len(empty) > 0 { + return fmt.Errorf("supplied empty values for keys not declared in service-secrets.may-be-empty: %s", strings.Join(empty, ", ")) + } + return nil +} + // fallbackSource is what produces a property no remote key holds: the // environment's own configuration, a declared generator, or the operator. // @@ -704,6 +827,15 @@ func (plan *Plan) Apply(ctx context.Context, store Store, allowMissing bool) ([] if err := plan.ValidateApply(allowMissing); err != nil { return nil, err } + if store.Name() != plan.Store { + return nil, fmt.Errorf("plan belongs to a different secret store") + } + // Check propagation sources as well as destinations before the first write. + for _, state := range plan.states { + if err := unchangedState(ctx, store, state); err != nil { + return nil, err + } + } var written []Written for i, write := range plan.writes { if err := unchanged(ctx, store, &write); err != nil { @@ -726,6 +858,29 @@ func (plan *Plan) Apply(ctx context.Context, store Store, allowMissing bool) ([] return written, nil } +// This is an optimistic check, not an atomic compare-and-set. Neither backend's +// version creation API offers that guarantee; operators must serialize writers. +func unchangedState(ctx context.Context, store Store, state *remoteState) error { + key := state.secret.RemoteKey + description, err := store.Describe(ctx, key) + if err != nil { + return wrapKeyError("recheck", key, err) + } + if description != state.description { + return fmt.Errorf("remote key %s changed since planning; rebuild the plan", key) + } + if description.HasVersion { + document, err := store.Read(ctx, key) + if err != nil { + return wrapKeyError("recheck", key, err) + } + if !maps.Equal(document, state.document) { + return fmt.Errorf("remote key %s changed since planning; rebuild the plan", key) + } + } + return nil +} + // unchanged refuses a write to a key that is no longer as the plan read it: a // document built from what it used to hold would silently undo whatever was // written since. The store offers no conditional version add, so this narrows diff --git a/pkg/deploysecrets/plan_test.go b/pkg/deploysecrets/plan_test.go index 0a8635f72..bf7bff201 100644 --- a/pkg/deploysecrets/plan_test.go +++ b/pkg/deploysecrets/plan_test.go @@ -320,13 +320,11 @@ func TestPlanRefusesToPropagateAConflictingValue(t *testing.T) { } // With nothing stored at all, a generated shared value is minted once and is -// one value everywhere it is read; a key that exists with no enabled version is -// written like a missing one. +// one value everywhere it is read. func TestPlanOnAnEmptyStoreGeneratesOnceAndAgrees(t *testing.T) { ctx := context.Background() rendered := fixture() store := newFakeStore(map[string]map[string]string{}) - store.empty["host-accounts"] = true plan, err := Build(ctx, &Inputs{Rendered: rendered, Generators: generators(), Services: []string{"tasks/store"}, Store: store, ReadPayloads: true}) if err != nil { @@ -343,7 +341,7 @@ func TestPlanOnAnEmptyStoreGeneratesOnceAndAgrees(t *testing.T) { t.Error("a generated shared value differs between the keys reading it") } if !slices.Contains(store.writes, "host-accounts") { - t.Error("a key with no enabled version was not written") + t.Error("the missing key was not written") } } @@ -704,7 +702,7 @@ func TestAnEd25519SigningKeyIsMintedOnceForItsOneReader(t *testing.T) { Configuration: "postgres", Services: []string{"tasks/store"}, Keys: []string{"POSTGRES_PASSWORD"}, Format: environments.SecretGeneratorFormatEd25519PKCS8}} plan, err := Build(ctx, &Inputs{Rendered: rendered, Generators: keyGenerator, - Services: []string{"tasks/store"}, Store: store, ReadPayloads: true}) + Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, MintKeys: []string{storePassword}}) if err != nil { t.Fatal(err) } diff --git a/pkg/deploysecrets/retained_material_test.go b/pkg/deploysecrets/retained_material_test.go new file mode 100644 index 000000000..882f86309 --- /dev/null +++ b/pkg/deploysecrets/retained_material_test.go @@ -0,0 +1,84 @@ +package deploysecrets + +import ( + "context" + "errors" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/stretchr/testify/require" +) + +type failOneWrite struct { + *fakeStore + key string +} + +func (store *failOneWrite) Write(ctx context.Context, key string, document map[string]string, create bool) (string, error) { + if key == store.key { + return "", errors.New("controlled write failure") + } + return store.fakeStore.Write(ctx, key, document, create) +} + +func TestPartialApplyReportsCompletedKeysAndRetryKeepsMaterial(t *testing.T) { + store := &failOneWrite{fakeStore: newFakeStore(seeded()), key: "tasks-store"} + inputs := &Inputs{Rendered: fixture(), Store: store, ReadPayloads: true, Generators: generators(), Services: []string{"tasks/store"}} + plan, err := Build(context.Background(), inputs) + require.NoError(t, err) + written, err := plan.Apply(context.Background(), store, true) + require.ErrorContains(t, err, "controlled write failure") + require.Len(t, written, 1) + require.Equal(t, "notes-backend", written[0].Key) + retained := store.documents["notes-backend"][internalToken] + store.key = "" + plan, err = Build(context.Background(), inputs) + require.NoError(t, err) + written, err = plan.Apply(context.Background(), store, true) + require.NoError(t, err) + require.Len(t, written, 1) + require.Equal(t, "tasks-store", written[0].Key) + require.Equal(t, retained, store.documents["notes-backend"][internalToken]) +} + +func TestPlannerRefusesExistingKeyWithoutReadableVersion(t *testing.T) { + for _, read := range []bool{false, true} { + store := newFakeStore(seeded()) + store.empty["host-accounts"] = true + _, err := Build(context.Background(), &Inputs{Rendered: fixture(), Store: store, ReadPayloads: read, Generators: generators()}) + require.ErrorContains(t, err, "without a readable version") + require.Empty(t, store.writes) + } +} + +func TestPlannerRefusesChangesBeforeApply(t *testing.T) { + for _, existing := range []bool{false, true} { + store := newFakeStore(seeded()) + if !existing { + delete(store.documents, "wiki-backend") + } + plan, err := Build(context.Background(), &Inputs{Rendered: fixture(), Store: store, ReadPayloads: true, Generators: generators()}) + require.NoError(t, err) + store.documents["wiki-backend"] = map[string]string{internalToken: "changed-after-confirmation"} + written, err := plan.Apply(context.Background(), store, true) + require.ErrorContains(t, err, "changed since planning") + require.Empty(t, written) + require.Empty(t, store.writes) + } +} + +func TestSigningKeyRequiresExplicitMintAndNeverRemintsExistingDocument(t *testing.T) { + generator := []environments.EnvironmentSecretGenerator{{Scope: environments.SecretGeneratorScopeService, + Configuration: "postgres", Services: []string{"tasks/store"}, Keys: []string{"POSTGRES_PASSWORD"}, + Format: environments.SecretGeneratorFormatEd25519PKCS8}} + store := newFakeStore(map[string]map[string]string{}) + inputs := &Inputs{Rendered: fixture(), Store: store, ReadPayloads: true, Generators: generator, Services: []string{"tasks/store"}} + plan, err := Build(context.Background(), inputs) + require.NoError(t, err) + require.Equal(t, ActionRequire, propertyPlan(t, plan, "tasks-store", storePassword).Action) + store.documents["tasks-store"] = map[string]string{"unrelated": "retained"} + inputs.MintKeys = []string{storePassword} + _, err = Build(context.Background(), inputs) + require.ErrorContains(t, err, "never re-mint") + require.Empty(t, store.writes) +} diff --git a/pkg/deploysecrets/rotate_test.go b/pkg/deploysecrets/rotate_test.go index 98e5dca31..db7bf232f 100644 --- a/pkg/deploysecrets/rotate_test.go +++ b/pkg/deploysecrets/rotate_test.go @@ -329,10 +329,10 @@ func TestRotateRefusesAKeyThatMovedSinceThePlan(t *testing.T) { } store.versions = map[string]int{consumerKey: 5} _, err = plan.Apply(context.Background(), store, false) - if err == nil || !strings.Contains(err.Error(), consumerKey+" changed after the plan read it") { + if err == nil || !strings.Contains(err.Error(), consumerKey+" changed since planning") { t.Fatalf("Apply = %v, want the moved key refused by name", err) } - if store.documents[evidenceKey][evidencePassword] != oldPassword || slices.Contains(store.writes[:1], consumerKey) { + if store.documents[evidenceKey][evidencePassword] != oldPassword || len(store.writes) != 0 { t.Error("the rotation was left half-applied or overwrote the moved key") } } diff --git a/pkg/deploysecrets/store.go b/pkg/deploysecrets/store.go index c233aa845..18c635820 100644 --- a/pkg/deploysecrets/store.go +++ b/pkg/deploysecrets/store.go @@ -38,10 +38,11 @@ type Store interface { // Description is what can be known about a remote key without reading it. type Description struct { Exists bool - // HasVersion is false for a key created with no enabled version: reading it - // fails, so it is planned as an empty document. + // HasVersion is false when no version can be read. Existing material must + // not be treated as absent in that case. HasVersion bool - // Version identifies the newest enabled version. Apply compares it before + // Version identifies the observed remote revision (a version ID or metadata + // fingerprint for backends without a metadata-only latest-version read). Apply compares it before // each write, so a key someone changed after the plan read it is refused // rather than overwritten with a document built from what it used to hold. Version string @@ -151,6 +152,12 @@ func ResolveStore(ctx context.Context, run Runner, target ClusterTarget, ref env if err := json.Unmarshal(output, &store); err != nil { return nil, fmt.Errorf("decode %s %s: invalid store document (backend output withheld)", ref.Kind, ref.Name) } + if len(store.Spec.Provider) != 1 { + return nil, fmt.Errorf("%s %s must declare exactly one secret provider", ref.Kind, ref.Name) + } + if raw, ok := store.Spec.Provider["azurekv"]; ok { + return resolveAzureStore(ctx, run, target, ref, namespace, raw) + } if raw, ok := store.Spec.Provider["gcpsm"]; ok { var gcp struct { ProjectID string `json:"projectID"` @@ -167,7 +174,7 @@ func ResolveStore(ctx context.Context, run Runner, target ClusterTarget, ref env for provider := range store.Spec.Provider { providers = append(providers, provider) } - return nil, fmt.Errorf("%s %s resolves through provider %v, which `codefly deploy secrets` cannot write yet (supported: gcpsm)", ref.Kind, ref.Name, providers) + return nil, fmt.Errorf("%s %s resolves through provider %v, which `codefly deploy secrets` cannot write yet (supported: gcpsm, azurekv)", ref.Kind, ref.Name, providers) } // GoogleSecretManager is a gcpsm-backed store, driven through the operator's own diff --git a/pkg/deploysecrets/supplied_test.go b/pkg/deploysecrets/supplied_test.go new file mode 100644 index 000000000..85e937616 --- /dev/null +++ b/pkg/deploysecrets/supplied_test.go @@ -0,0 +1,95 @@ +package deploysecrets + +import ( + "context" + "strings" + "testing" +) + +const externalValue = "operator-supplied-secret" + +// The staging gap: an external client secret nothing can generate is handed in +// by the operator and written, while the rest of the plan is unchanged. +func TestASuppliedValueFillsARequiredProperty(t *testing.T) { + ctx := context.Background() + rendered := fixture() + store := newFakeStore(map[string]map[string]string{}) + supplied := map[string]string{clientSecret: externalValue} + if err := ValidateSupplied(rendered, supplied, nil); err != nil { + t.Fatal(err) + } + plan, err := Build(ctx, &Inputs{Rendered: rendered, Generators: generators(), + Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, Supplied: supplied}) + if err != nil { + t.Fatal(err) + } + got := propertyPlan(t, plan, "host-accounts", clientSecret) + if got.Action != ActionSupply || strings.Contains(got.Source, externalValue) { + t.Fatalf("plan = %+v, want supply without the value", got) + } + if _, err := plan.Apply(ctx, store, true); err != nil { + t.Fatal(err) + } + if store.documents["host-accounts"][clientSecret] != externalValue { + t.Error("the supplied value was not written") + } + // A rerun with the same value keeps it. + plan, err = Build(ctx, &Inputs{Rendered: rendered, Generators: generators(), + Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, Supplied: supplied}) + if err != nil { + t.Fatal(err) + } + if got := propertyPlan(t, plan, "host-accounts", clientSecret); got.Action != ActionKeep { + t.Errorf("rerun = %s, want keep", got.Action) + } +} + +func TestASuppliedValueNeverReplacesAStoredOne(t *testing.T) { + _, err := Build(context.Background(), &Inputs{Rendered: fixture(), Generators: generators(), + Services: []string{"tasks/store"}, Store: newFakeStore(seeded()), ReadPayloads: true, + Supplied: map[string]string{clientSecret: externalValue}}) + if err == nil || !strings.Contains(err.Error(), "never replaces") || strings.Contains(err.Error(), externalValue) { + t.Fatalf("Build = %v, want the stored value kept and the conflict refused", err) + } +} + +func TestASuppliedValueNeverStandsInForAGenerator(t *testing.T) { + _, err := Build(context.Background(), &Inputs{Rendered: fixture(), Generators: generators(), + Services: []string{"tasks/store"}, Store: newFakeStore(map[string]map[string]string{}), ReadPayloads: true, + Supplied: map[string]string{internalToken: externalValue}}) + if err == nil || !strings.Contains(err.Error(), "service-secrets.generate") { + t.Fatalf("Build = %v, want a supplied generated key refused", err) + } +} + +func TestSuppliedValuesMustBeReadAndNonEmptyUnlessDeclared(t *testing.T) { + rendered := fixture() + if err := ValidateSupplied(rendered, map[string]string{clientSecret + "_TYPO": "x"}, nil); err == nil { + t.Error("a key no property reads was accepted") + } + if err := ValidateSupplied(rendered, map[string]string{clientSecret: ""}, nil); err == nil { + t.Error("an undeclared empty value was accepted") + } + if err := ValidateSupplied(rendered, map[string]string{clientSecret: ""}, []string{clientSecret}); err != nil { + t.Errorf("a declared empty value was refused: %v", err) + } +} + +// A declared-empty key is written empty, so the ExternalSecret that reads it +// resolves instead of failing on a missing property. +func TestADeclaredEmptySuppliedValueIsWritten(t *testing.T) { + ctx := context.Background() + store := newFakeStore(map[string]map[string]string{}) + plan, err := Build(ctx, &Inputs{Rendered: fixture(), Generators: generators(), + Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, + MayBeEmpty: []string{clientSecret}, Supplied: map[string]string{clientSecret: ""}}) + if err != nil { + t.Fatal(err) + } + if _, err := plan.Apply(ctx, store, true); err != nil { + t.Fatal(err) + } + if value, present := store.documents["host-accounts"][clientSecret]; !present || value != "" { + t.Errorf("declared-empty property = %q, %v; want written empty", value, present) + } +} diff --git a/pkg/environments/environment.go b/pkg/environments/environment.go index 18fefcc4f..989500b43 100644 --- a/pkg/environments/environment.go +++ b/pkg/environments/environment.go @@ -1236,6 +1236,18 @@ type SecretScope struct { Service string } +// MappingFor selects a module-qualified declaration before the unqualified fallback, +// matching service configuration and identity selection. +func (s *EnvironmentServiceSecrets) MappingFor(module, service string) EnvironmentServiceSecretMapping { + if s == nil { + return EnvironmentServiceSecretMapping{} + } + if mapping, ok := s.Services[module+"/"+service]; module != "" && ok { + return mapping + } + return s.Services[service] +} + // RemoteRef locates one secret key of a service in the remote store. An explicit // per-service RemoteKeys entry wins; else the service's own Defaults template; // else the environment-wide Defaults template; else the "/" store @@ -1244,7 +1256,7 @@ type SecretScope struct { // fallback path, the same as an environment that declares nothing. func (s *EnvironmentServiceSecrets) RemoteRef(scope SecretScope, key string) EnvironmentSecretRemoteRef { if s != nil { - mapping := s.Services[scope.Service] + mapping := s.MappingFor(scope.Module, scope.Service) if remote, ok := mapping.RemoteKeys[key]; ok { return remote } diff --git a/pkg/environments/environment_test.go b/pkg/environments/environment_test.go index b641f10cb..4a504f54d 100644 --- a/pkg/environments/environment_test.go +++ b/pkg/environments/environment_test.go @@ -90,10 +90,11 @@ func serviceIdentityDeclaration(serviceName string) string { // service passes the graph check. func TestValidateEnvironmentsAllowsKnownService(t *testing.T) { for name, declaration := range map[string]string{ - "service-secrets": serviceSecretsDeclaration("accounts"), - "service-config": serviceConfigDeclaration("accounts"), - "qualified service-config": serviceConfigDeclaration("saas/accounts"), - "service-identity": serviceIdentityDeclaration("accounts"), + "service-secrets": serviceSecretsDeclaration("accounts"), + "qualified service-secrets": serviceSecretsDeclaration("saas/accounts"), + "service-config": serviceConfigDeclaration("accounts"), + "qualified service-config": serviceConfigDeclaration("saas/accounts"), + "service-identity": serviceIdentityDeclaration("accounts"), } { t.Run(name, func(t *testing.T) { ctx := context.Background() @@ -113,10 +114,11 @@ func TestValidateEnvironmentsAllowsKnownService(t *testing.T) { // projection time; the graph check must reject it. func TestValidateEnvironmentsRejectsUnknownService(t *testing.T) { for name, declaration := range map[string]string{ - "service-secrets": serviceSecretsDeclaration("accunts"), - "service-config": serviceConfigDeclaration("accunts"), - "qualified service-config": serviceConfigDeclaration("wrong/accounts"), - "service-identity": serviceIdentityDeclaration("accunts"), + "service-secrets": serviceSecretsDeclaration("accunts"), + "qualified service-secrets": serviceSecretsDeclaration("wrong/accounts"), + "service-config": serviceConfigDeclaration("accunts"), + "qualified service-config": serviceConfigDeclaration("wrong/accounts"), + "service-identity": serviceIdentityDeclaration("accunts"), } { t.Run(name, func(t *testing.T) { ctx := context.Background() diff --git a/pkg/environments/workspace.go b/pkg/environments/workspace.go index 8e5807bbd..d343be58a 100644 --- a/pkg/environments/workspace.go +++ b/pkg/environments/workspace.go @@ -191,7 +191,7 @@ func validateResourcePathComponent(kind, name string) error { // it earlier. // Loading the graph is why this is a pass separate from postLoad, mirroring // ValidateServiceDependencies. -func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace) error { +func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, selected ...*Environment) error { environments, err := FromWorkspace(workspace) if err != nil { return err @@ -200,6 +200,17 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace) erro if nsErr := validateModuleNamespaces(workspace, environments); nsErr != nil { return w.Wrap(nsErr) } + if len(selected) > 0 { + for _, env := range selected { + if env == nil { + return fmt.Errorf("selected environment is nil") + } + if err := env.Validate(); err != nil { + return err + } + } + environments = selected + } for _, env := range environments { if env == nil { continue @@ -235,7 +246,7 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace) erro // A module-qualified service-identity key names exactly one // service of that module, or nothing (a typo that would leave // the workload on the default identity with no report). - if _, service, qualified := strings.Cut(name, "/"); qualified && (block == "service-identity" || block == "service-mesh" || block == "service-config") { + if _, service, qualified := strings.Cut(name, "/"); qualified { if !slices.Contains(byName[service], name) { return w.Wrap(fmt.Errorf("environment %q %s references unknown service %q", env.Name, block, name)) } diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index 3e6b09a0e..386d94d56 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -100,16 +100,39 @@ func RenderCell(ctx context.Context, workspace *resources.Workspace, env *enviro return result, nil } +// RenderPublishedCell inventories the committed publication trees without rebuilding +// images or rewriting their signatures. Used when binding an assembled deployment branch. +func RenderPublishedCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (CellResult, error) { + if workspace == nil || env == nil { + return CellResult{}, errors.New("a cell is rendered for a workspace and an environment") + } + root, err := cellRoot(workspace, env) + if err != nil { + return CellResult{}, err + } + file, result, err := deriveCellFrom(ctx, workspace, env, filepath.Join(root, "modules")) + if err != nil { + return CellResult{}, err + } + return result, writeCell(&file, result.Path) +} + // deriveCell is RenderCell without the write: the cell file the rendered // trees and the composition describe, and the path it belongs at. func deriveCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (cellfile.File, CellResult, error) { + return deriveCellFrom(ctx, workspace, env, "") +} + +func deriveCellFrom(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, modulesDir string) (cellfile.File, CellResult, error) { if workspace == nil || env == nil { return cellfile.File{}, CellResult{}, errors.New("a cell is rendered for a workspace and an environment") } if env.Host == nil { return cellfile.File{}, CellResult{}, fmt.Errorf("environment %s declares no host, so it has no cell to describe", env.Name) } - modulesDir := filepath.Dir(moduleRenderDestination(workspace, "module")) + if modulesDir == "" { + modulesDir = filepath.Dir(moduleRenderDestination(workspace, "module")) + } entries, err := os.ReadDir(modulesDir) if errors.Is(err, fs.ErrNotExist) { return cellfile.File{}, CellResult{}, fmt.Errorf("no module has been rendered under %s", modulesDir) diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index 1a171ad9f..25515d412 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -210,6 +210,30 @@ func renderCellTreeWith(t *testing.T, options *RenderOptions, service, environme // container port the mesh authorizes and the Service port an edge route uses, // who the composition says calls them, the ingress into them, the bindings // the environment declares, and the egress a service is granted. +func TestPublishedCellUsesPublishedTreesAndRefusesMissingModules(t *testing.T) { + workspace := writeCellWorkspace(t) + env := selectedEnvironment(t, workspace, "staging") + renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + env.Gitops = &environments.EnvironmentGitops{RepoURL: "https://github.com/example/product", Branch: "deploy", Path: "deployments/published"} + publication := filepath.Join(workspace.Dir(), env.Gitops.Path) + require.NoError(t, os.MkdirAll(publication, 0o755)) + require.NoError(t, os.Rename(filepath.Join(workspace.Dir(), "deployments/modules"), filepath.Join(publication, "modules"))) + result, err := RenderPublishedCell(context.Background(), workspace, env) + require.NoError(t, err) + require.Equal(t, []string{"billing", "shop"}, result.Modules) + before, err := os.ReadFile(result.Path) + require.NoError(t, err) + _, err = RenderCell(context.Background(), workspace, env) + require.ErrorContains(t, err, "no module has been rendered") + require.NoError(t, os.RemoveAll(filepath.Join(publication, "modules/billing"))) + _, err = RenderPublishedCell(context.Background(), workspace, env) + require.ErrorContains(t, err, "billing") + after, err := os.ReadFile(result.Path) + require.NoError(t, err) + require.Equal(t, before, after, "incomplete publication must leave the last complete cell unchanged") +} + func TestCellFileInventoriesEveryRenderedWorkload(t *testing.T) { workspace := writeCellWorkspace(t) env := selectedEnvironment(t, workspace, "staging") diff --git a/pkg/gitops/configuration_projection.go b/pkg/gitops/configuration_projection.go index 97a7b0bbf..45e45e977 100644 --- a/pkg/gitops/configuration_projection.go +++ b/pkg/gitops/configuration_projection.go @@ -35,7 +35,7 @@ func projectConfigurationValuesInTree(ctx context.Context, tree *renderRoot, roo values := env.ServiceConfig.ValuesFor(module, service) refs := map[string]environments.EnvironmentSecretRemoteRef{} if env.ServiceSecrets != nil { - refs = env.ServiceSecrets.Services[service].RemoteKeys + refs = env.ServiceSecrets.MappingFor(module, service).RemoteKeys } if len(values) == 0 && len(refs) == 0 { return nil @@ -190,7 +190,7 @@ func validateProjectedConfigurationInTree(tree *renderRoot, root string, service values := env.ServiceConfig.ValuesFor(scope.Module, service.Name) refs := map[string]environments.EnvironmentSecretRemoteRef{} if env.ServiceSecrets != nil { - refs = env.ServiceSecrets.Services[service.Name].RemoteKeys + refs = env.ServiceSecrets.MappingFor(scope.Module, service.Name).RemoteKeys } identity, err := soleWorkloadIdentity(scope.Module, service.Name, consumedManagedServices(service, scope.Module, env), env) if err != nil { diff --git a/pkg/gitops/configuration_projection_test.go b/pkg/gitops/configuration_projection_test.go index 2baab6e83..58eae6d5e 100644 --- a/pkg/gitops/configuration_projection_test.go +++ b/pkg/gitops/configuration_projection_test.go @@ -439,3 +439,46 @@ func TestRejectedConfigurationCannotReplacePublishedTree(t *testing.T) { require.NoError(t, err) require.Len(t, entries, 1) } + +func TestQualifiedSecretsReachOnlyTheirModule(t *testing.T) { + env := &environments.Environment{Name: "blue", Namespace: "product", ServiceSecrets: &environments.EnvironmentServiceSecrets{ + SecretStore: environments.EnvironmentSecretStoreReference{Name: "default-store", Kind: "ClusterSecretStore"}, + Services: map[string]environments.EnvironmentServiceSecretMapping{ + "saas/accounts": { + SecretStore: &environments.EnvironmentSecretStoreReference{Name: "accounts-store", Kind: "ClusterSecretStore"}, + RefreshInterval: "2m", + RemoteKeys: map[string]environments.EnvironmentSecretRemoteRef{"SIGNING_KEY": {Key: "accounts-signing", Property: "wrapped"}}, + }, + }, + }} + for _, module := range []string{"saas", "other"} { + t.Run(module, func(t *testing.T) { + root := t.TempDir() + writeConsumerTree(t, root, env.Name, env.Namespace, "accounts", "declared.example") + scope := scopeOf(env) + scope.Module = module + require.NoError(t, projectServiceConfiguration(t.Context(), testRenderRoot(t, root), root, &resources.Service{Name: "accounts"}, env, scope, serviceInjection{})) + rendered := buildOverlay(t, root, env.Name) + values := containerEnvironment(t, rendered) + if module == "other" { + require.NotContains(t, values, "SIGNING_KEY") + return + } + require.Equal(t, map[string]any{"secretKeyRef": map[string]any{"name": "secret-accounts", "key": "SIGNING_KEY"}}, values["SIGNING_KEY"]["valueFrom"]) + found := false + for _, doc := range rendered { + if doc.kind != "ExternalSecret" { + continue + } + spec := doc.value["spec"].(map[string]any) + require.Equal(t, "accounts-store", spec["secretStoreRef"].(map[string]any)["name"]) + require.Equal(t, "2m", spec["refreshInterval"]) + found = true + } + require.True(t, found) + remote := env.ServiceSecrets.RemoteRef(environments.SecretScope{Module: module, Service: "accounts"}, "SIGNING_KEY") + require.Equal(t, "accounts-signing", remote.Key) + require.Equal(t, "wrapped", remote.Property) + }) + } +} diff --git a/pkg/gitops/configuration_templates.go b/pkg/gitops/configuration_templates.go index cd79daa6b..feb3a0059 100644 --- a/pkg/gitops/configuration_templates.go +++ b/pkg/gitops/configuration_templates.go @@ -196,7 +196,7 @@ func producerPrimitiveRemote( // The producer's own projection takes its per-service override the same // way (serviceSecretProjection), so this is the store its ExternalSecret // actually reads from. - if override := secrets.Services[service].SecretStore; override != nil { + if override := secrets.MappingFor(module, service).SecretStore; override != nil { store = *override } } diff --git a/pkg/gitops/dev.go b/pkg/gitops/dev.go index 1d0416a04..d9dc64021 100644 --- a/pkg/gitops/dev.go +++ b/pkg/gitops/dev.go @@ -134,7 +134,7 @@ func buildRenderedServiceImages( rebuild bool, sink orchestration.OutputSink, ) ([]string, error) { - if err := environments.ValidateWorkspace(ctx, workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, workspace, env); err != nil { return nil, err } if err := prepareSnapshotRegistry(ctx, env); err != nil { diff --git a/pkg/gitops/frontend_payload.go b/pkg/gitops/frontend_payload.go index ba1c7eaf3..f1ea25367 100644 --- a/pkg/gitops/frontend_payload.go +++ b/pkg/gitops/frontend_payload.go @@ -11,6 +11,7 @@ import ( clicomposition "github.com/codefly-dev/cli/pkg/composition" "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/cli/pkg/orchestration" "github.com/codefly-dev/cli/pkg/solutionrun" "github.com/codefly-dev/core/composition" basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" @@ -109,7 +110,13 @@ func artifactCacheInjection(service *resources.Service, env *environments.Enviro // buildFrontendBundle and publishRuntimeArtifact are the build and the push a // test stands in for, the way stubBuild stands in for an image build. var ( - buildFrontendBundle = solutionrun.BuildFrontendBundle + buildFrontendBundle = func(ctx context.Context, module *resources.Module, env *environments.Environment, destination string) error { + builder, err := orchestration.EnsureEnvironmentBuilder(ctx, env) + if err != nil { + return err + } + return solutionrun.BuildFrontendBundleWithBuilder(ctx, module, destination, builder) + } publishRuntimeArtifact = clicomposition.PublishRuntimeArtifact ) @@ -141,7 +148,7 @@ func publishFrontendBundle(ctx context.Context, workspace *resources.Workspace, } defer func() { _ = os.RemoveAll(scratch) }() bundle := filepath.Join(scratch, "bundle") - if err = buildFrontendBundle(ctx, module, bundle); err != nil { + if err = buildFrontendBundle(ctx, module, env, bundle); err != nil { return nil, err } archive, _, err := composition.CanonicalArchive(bundle) diff --git a/pkg/gitops/frontend_publish_test.go b/pkg/gitops/frontend_publish_test.go index 440d1fc83..17a2d9bb0 100644 --- a/pkg/gitops/frontend_publish_test.go +++ b/pkg/gitops/frontend_publish_test.go @@ -24,7 +24,7 @@ func stubFrontendPublication(t *testing.T) (*int, *string) { builds, published := 0, "" previousBuild, previousPublish := buildFrontendBundle, publishRuntimeArtifact t.Cleanup(func() { buildFrontendBundle, publishRuntimeArtifact = previousBuild, previousPublish }) - buildFrontendBundle = func(_ context.Context, _ *resources.Module, destination string) error { + buildFrontendBundle = func(_ context.Context, _ *resources.Module, _ *environments.Environment, destination string) error { builds++ require.NoError(t, os.MkdirAll(destination, 0o755)) return os.WriteFile(filepath.Join(destination, solutionrun.FrontendBundleManifest), []byte("{}"), 0o600) diff --git a/pkg/gitops/orchestrate.go b/pkg/gitops/orchestrate.go index 45ae2375d..1b99a5421 100644 --- a/pkg/gitops/orchestrate.go +++ b/pkg/gitops/orchestrate.go @@ -46,7 +46,7 @@ func renderModuleTree( if err := selectionguard.RejectUnboundExecution(workspace.Dir(), module.Dir()); err != nil { return RenderResult{}, err } - if err := environments.ValidateWorkspace(ctx, workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, workspace, env); err != nil { return RenderResult{}, err } if env.WithdrawsModule(module) { @@ -618,7 +618,7 @@ func RenderService(ctx context.Context, workspace *resources.Workspace, module * } func renderService(ctx context.Context, workspace *resources.Workspace, module *resources.Module, service *resources.Service, env *environments.Environment, project string, build renderBuild, sink orchestration.OutputSink) (RenderResult, error) { - if err := environments.ValidateWorkspace(ctx, workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, workspace, env); err != nil { return RenderResult{}, err } if err := CheckCompositionPermissions(ctx, workspace, env, true, module.Name); err != nil { diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index e8c2d803a..1f7e58ee8 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -2201,6 +2201,7 @@ func openOrUpdatePullRequest(ctx context.Context, prepared *preparedRepository, Head: github.Ptr(prepared.plan.PromotionBranch), Base: github.Ptr(prepared.plan.BaseBranch), Body: github.Ptr(body), + Draft: github.Ptr(true), }) if err != nil { return "", 0, fmt.Errorf("open promotion pull request: %w", err) diff --git a/pkg/gitops/secret_projection.go b/pkg/gitops/secret_projection.go index 7865995ef..c7d8977c6 100644 --- a/pkg/gitops/secret_projection.go +++ b/pkg/gitops/secret_projection.go @@ -140,7 +140,7 @@ func serviceSecretProjection(scope unitScope, service string, secrets *environme return nil, nil } namespace := scope.Namespace - mapping := secrets.Services[service] + mapping := secrets.MappingFor(scope.Module, service) // A service may resolve from a different store than the environment default, // mirroring EnvironmentManagedSecretReference's per-reference store; without // this the per-service secret-store declaration would load, validate, and then diff --git a/pkg/gitops/secret_projection_test.go b/pkg/gitops/secret_projection_test.go index 0aba59b32..abe24cd28 100644 --- a/pkg/gitops/secret_projection_test.go +++ b/pkg/gitops/secret_projection_test.go @@ -248,6 +248,35 @@ func TestServiceSecretProjectionDefaultsAndOverridesRemoteKeys(t *testing.T) { } } +func TestServiceSecretProjectionQualifiedMappingDoesNotLeakAcrossModules(t *testing.T) { + secrets := azureServiceSecrets() + secrets.Services["saas/accounts"] = environments.EnvironmentServiceSecretMapping{ + SecretStore: &environments.EnvironmentSecretStoreReference{Name: "signing", Kind: "ClusterSecretStore"}, + RemoteKeys: map[string]environments.EnvironmentSecretRemoteRef{ + "workos-client-secret": {Key: "accounts-signing", Property: "wrapped"}, + }, + Defaults: &environments.EnvironmentSecretRemoteRef{Key: "{module}-{service}", Property: "{key}"}, + RefreshInterval: "5m", + } + for _, module := range []string{"saas", "other", "saas"} { + projection, err := serviceSecretProjection(unitScope{Namespace: "product-" + module, Module: module}, + "accounts", secrets, []string{"workos-api-key", "workos-client-secret"}) + if err != nil { + t.Fatal(err) + } + if module == "saas" { + if projection.Spec.SecretStoreRef.Name != "signing" || projection.Spec.RefreshInterval != "5m" || + projection.Spec.Data[0].RemoteRef.Key != "saas-accounts" || + projection.Spec.Data[1].RemoteRef != (externalSecretRemote{Key: "accounts-signing", Property: "wrapped"}) { + t.Fatalf("qualified mapping lost: %+v", projection.Spec) + } + } else if projection.Spec.SecretStoreRef.Name != "azure-keyvault-prod" || + projection.Spec.Data[1].RemoteRef.Key != "workos/prod/client-secret" { + t.Fatalf("qualified mapping leaked to another module: %+v", projection.Spec) + } + } +} + func TestServiceSecretProjectionRendersNothingWithoutStoreOrKeys(t *testing.T) { if projection, err := serviceSecretProjection(unitScope{Namespace: "payments"}, "accounts", nil, []string{"api-key"}); err != nil || projection != nil { t.Fatalf("nil store rendered %v (err %v)", projection, err) diff --git a/pkg/gitops/solution.go b/pkg/gitops/solution.go index ad7b3e72c..ab82414bc 100644 --- a/pkg/gitops/solution.go +++ b/pkg/gitops/solution.go @@ -59,7 +59,7 @@ func RenderSolution(ctx context.Context, req *SolutionRenderRequest) (RenderResu if err := selectionguard.RejectUnboundExecution(req.Workspace.Dir(), req.Source); err != nil { return RenderResult{}, err } - if err := environments.ValidateWorkspace(ctx, req.Workspace); err != nil { + if err := environments.ValidateWorkspace(ctx, req.Workspace, req.Environment); err != nil { return RenderResult{}, err } env := req.Environment diff --git a/pkg/internal/azureauth/credential.go b/pkg/internal/azureauth/credential.go new file mode 100644 index 000000000..21359e9b5 --- /dev/null +++ b/pkg/internal/azureauth/credential.go @@ -0,0 +1,50 @@ +// Package azureauth selects one explicit Azure identity for publication. +package azureauth + +import ( + "errors" + "os" + "path/filepath" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/google/uuid" +) + +// Credential never falls back between an operator login and workload identity. +// The caller supplies the tenant declared by the destination, not an ambient default. +func Credential(tenant string, lookup func(string) (string, bool)) (azcore.TokenCredential, error) { + if lookup == nil { + lookup = os.LookupEnv + } + id, err := uuid.Parse(tenant) + if err != nil || id == uuid.Nil || id.String() != tenant { + return nil, errors.New("Azure publication requires a declared tenant UUID") + } + mode, _ := lookup("CODEFLY_AZURE_CREDENTIAL") + switch mode { + case "", "azure-cli": + credential, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{TenantID: tenant}) + if err != nil { + return nil, errors.New("cannot initialize the Azure CLI publication credential") + } + return credential, nil + case "workload": + workloadTenant, _ := lookup("AZURE_TENANT_ID") + client, _ := lookup("AZURE_CLIENT_ID") + tokenFile, _ := lookup("AZURE_FEDERATED_TOKEN_FILE") + clientID, err := uuid.Parse(client) + if workloadTenant != tenant || err != nil || clientID == uuid.Nil || clientID.String() != client || !filepath.IsAbs(tokenFile) { + return nil, errors.New("Azure workload publication requires the declared tenant, a client UUID and an absolute federated token file") + } + credential, err := azidentity.NewWorkloadIdentityCredential(&azidentity.WorkloadIdentityCredentialOptions{ + TenantID: tenant, ClientID: client, TokenFilePath: tokenFile, + }) + if err != nil { + return nil, errors.New("cannot initialize the Azure workload publication credential") + } + return credential, nil + default: + return nil, errors.New("CODEFLY_AZURE_CREDENTIAL must be azure-cli or workload") + } +} diff --git a/pkg/internal/azureauth/credential_test.go b/pkg/internal/azureauth/credential_test.go new file mode 100644 index 000000000..fcae572b3 --- /dev/null +++ b/pkg/internal/azureauth/credential_test.go @@ -0,0 +1,47 @@ +package azureauth + +import ( + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/stretchr/testify/require" +) + +const tenant = "11111111-1111-1111-1111-111111111111" + +func TestCredentialUsesOnlyTheSelectedIdentity(t *testing.T) { + values := map[string]string{ + "AZURE_TENANT_ID": tenant, "AZURE_CLIENT_ID": "22222222-2222-2222-2222-222222222222", + "AZURE_FEDERATED_TOKEN_FILE": "/nonexistent/projected-token", + } + lookup := func(key string) (string, bool) { value, ok := values[key]; return value, ok } + credential, err := Credential(tenant, lookup) + require.NoError(t, err) + require.IsType(t, &azidentity.AzureCLICredential{}, credential) + values["CODEFLY_AZURE_CREDENTIAL"] = "workload" + credential, err = Credential(tenant, lookup) + require.NoError(t, err) + require.IsType(t, &azidentity.WorkloadIdentityCredential{}, credential) +} + +func TestWorkloadCredentialRefusesIncompleteOrCrossTenantIdentity(t *testing.T) { + for _, change := range []map[string]string{ + {"AZURE_TENANT_ID": "33333333-3333-3333-3333-333333333333"}, + {"AZURE_TENANT_ID": ""}, {"AZURE_CLIENT_ID": ""}, {"AZURE_CLIENT_ID": "private-marker"}, + {"AZURE_FEDERATED_TOKEN_FILE": ""}, {"AZURE_FEDERATED_TOKEN_FILE": "relative-token"}, + {"CODEFLY_AZURE_CREDENTIAL": "private-marker"}, + } { + values := map[string]string{ + "CODEFLY_AZURE_CREDENTIAL": "workload", "AZURE_TENANT_ID": tenant, + "AZURE_CLIENT_ID": "22222222-2222-2222-2222-222222222222", + "AZURE_FEDERATED_TOKEN_FILE": "/nonexistent/projected-token", + } + for key, value := range change { + values[key] = value + } + credential, err := Credential(tenant, func(key string) (string, bool) { value, ok := values[key]; return value, ok }) + require.Error(t, err) + require.Nil(t, credential) + require.NotContains(t, err.Error(), "private-marker") + } +} diff --git a/pkg/orchestration/build_plan.go b/pkg/orchestration/build_plan.go index 7b1665d9a..2b27517a5 100644 --- a/pkg/orchestration/build_plan.go +++ b/pkg/orchestration/build_plan.go @@ -564,12 +564,10 @@ func (b *Builder) buildRecipe( // CLI's builder, provisioned by dockerBuildContext before the agent's Build // RPC, so it is named rather than provisioned again here. // -// Beyond a multi-platform build, which the default "docker" driver cannot -// produce at all, a declared registry mirror also requires the CLI's own -// builder whatever the platforms: the default builder takes no buildkitd -// configuration, so a single-platform build left on it would read its base from -// the registry the recipe names and the mirror would reach the resolution -// alone. +// Hosted image builds use the same host-networked builder as mirrored builds: +// even a single-platform image without mirrors must resolve package repositories +// through the operator's network. Local builds without a registry keep Docker's +// default builder. func (b *Builder) selectImageBuilder(ctx context.Context, multiArch bool, mirrors map[string]string) (string, error) { if name := b.world.BuildxBuilder; name != "" { if len(mirrors) > 0 { @@ -590,7 +588,8 @@ func (b *Builder) selectImageBuilder(ctx context.Context, multiArch bool, mirror } return buildxBuilderFor(mirrors), nil } - if !multiArch && len(mirrors) == 0 { + hostedRegistry := b.world.Env != nil && b.world.Env.Registry != nil + if !multiArch && len(mirrors) == 0 && !hostedRegistry { return "", nil } return ensureBuildxBuilder(ctx, mirrors) diff --git a/pkg/orchestration/image_cache.go b/pkg/orchestration/image_cache.go index 5e0ba2fa9..b4861996a 100644 --- a/pkg/orchestration/image_cache.go +++ b/pkg/orchestration/image_cache.go @@ -726,17 +726,15 @@ func hashContextEntry(hasher hash.Hash, relative, path string, entry fs.DirEntry writeImageCacheRecord(hasher, "symlink", relative, filepath.ToSlash(target)) return nil case info.IsDir(): - writeImageCacheRecord(hasher, "directory", relative) + writeImageCacheRecord(hasher, "directory", relative, strconv.FormatUint(uint64(info.Mode().Perm()), 8)) return nil case !info.Mode().IsRegular(): writeImageCacheRecord(hasher, "special", relative, info.Mode().String()) return nil } - executable := "0" - if info.Mode().Perm()&0o111 != 0 { - executable = "1" - } - writeImageCacheRecord(hasher, "file", relative, executable, strconv.FormatInt(info.Size(), 10)) + // COPY preserves read and traversal permissions too: identical bytes can + // still be unreadable to the image's non-root user. + writeImageCacheRecord(hasher, "file", relative, strconv.FormatUint(uint64(info.Mode().Perm()), 8), strconv.FormatInt(info.Size(), 10)) file, err := os.Open(path) if err != nil { return err diff --git a/pkg/orchestration/image_cache_test.go b/pkg/orchestration/image_cache_test.go index 41f1d22dd..268eab7c3 100644 --- a/pkg/orchestration/image_cache_test.go +++ b/pkg/orchestration/image_cache_test.go @@ -370,6 +370,12 @@ func TestEveryContextChangeMovesTheImageIdentity(t *testing.T) { "mode changed": func(t *testing.T, harness *imageCacheHarness) { require.NoError(t, os.Chmod(filepath.Join(harness.serviceDir, "code", "main.go"), 0o755)) }, + "file readability changed": func(t *testing.T, harness *imageCacheHarness) { + require.NoError(t, os.Chmod(filepath.Join(harness.serviceDir, "code", "main.go"), 0o600)) + }, + "directory traversal changed": func(t *testing.T, harness *imageCacheHarness) { + require.NoError(t, os.Chmod(filepath.Join(harness.serviceDir, "code"), 0o700)) + }, "file replaced by a symlink": func(t *testing.T, harness *imageCacheHarness) { path := filepath.Join(harness.serviceDir, "code", "main.go") require.NoError(t, os.Remove(path)) diff --git a/pkg/orchestration/registry_mirror.go b/pkg/orchestration/registry_mirror.go index 7dbc394b1..f440492c5 100644 --- a/pkg/orchestration/registry_mirror.go +++ b/pkg/orchestration/registry_mirror.go @@ -1,6 +1,7 @@ package orchestration import ( + "context" "crypto/sha256" "encoding/hex" "fmt" @@ -48,9 +49,13 @@ func registryMirrors(env *environments.Environment) map[string]string { return env.Registry.Mirrors } -// mirroredReference is the reference to read `reference` from. It is -// environments.MirroredReference, the one spelling a cell's pull repository -// (EnvironmentRegistry.PullRepository) is mirrored with too. +// EnsureEnvironmentBuilder reuses the environment's registry configuration and +// host-networked builder for auxiliary builds such as frontend bundles. +func EnsureEnvironmentBuilder(ctx context.Context, env *environments.Environment) (string, error) { + return ensureBuildxBuilder(ctx, registryMirrors(env)) +} + +// mirroredReference uses the environment's declared read repository. func mirroredReference(ref string, mirrors map[string]string) string { return environments.MirroredReference(ref, mirrors) } diff --git a/pkg/solutionrun/frontend_build.go b/pkg/solutionrun/frontend_build.go index b958a8ade..11d6dc998 100644 --- a/pkg/solutionrun/frontend_build.go +++ b/pkg/solutionrun/frontend_build.go @@ -111,7 +111,16 @@ func BuildFrontendBundle(ctx context.Context, module *resources.Module, destinat return buildFrontendBundle(ctx, module, destination, frontendBuildContext) } -func buildFrontendBundle(ctx context.Context, module *resources.Module, destination string, stage func(context.Context, *resources.Module, string) (string, string, error)) error { +// BuildFrontendBundleWithBuilder uses the deployment environment's builder. +func BuildFrontendBundleWithBuilder(ctx context.Context, module *resources.Module, destination, builder string) error { + return buildFrontendBundle(ctx, module, destination, frontendBuildContext, builder) +} + +func buildFrontendBundle(ctx context.Context, module *resources.Module, destination string, stage func(context.Context, *resources.Module, string) (string, string, error), builders ...string) error { + builder := "" + if len(builders) > 0 { + builder = builders[0] + } project, err := FrontendProject(module) if err != nil { return err @@ -140,7 +149,13 @@ func buildFrontendBundle(ctx context.Context, module *resources.Module, destinat dockerfile := fmt.Sprintf("FROM %s AS build\nWORKDIR /source\nCOPY [\".\", \".\"]\nWORKDIR %s\nRUN [\"sh\", \"-c\", \"npm ci && npm run build\"]\nFROM scratch\nCOPY --from=build [\"%s/\", \"/\"]\n", FrontendBuilderImage, workdir, bundle) //nolint:gosec // G204: fixed docker executable, separate arguments, no shell; the Dockerfile arrives on stdin. - command := exec.CommandContext(ctx, "docker", "build", "--progress=plain", "--file", "-", "--output", "type=local,dest="+destination, root) + args := []string{"build"} + if builder != "" { + args = []string{"buildx", "build", "--builder", builder} + } + args = append(args, "--progress=plain", "--file", "-", "--output", "type=local,dest="+destination, root) + //nolint:gosec // G204: fixed docker executable, separate arguments, no shell. + command := exec.CommandContext(ctx, "docker", args...) command.Stdin = strings.NewReader(dockerfile) var output bytes.Buffer command.Stdout, command.Stderr = &output, &output From 1f33fc7eb65ece3810d4bff0d790967416a7d96d Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:36:11 +0000 Subject: [PATCH 2/6] fix(azure): confine profile imports and reuse planner validation --- cmd/environment/configuration_profile.go | 37 ++++++++++------ cmd/environment/import.go | 54 ++++++++++++++---------- pkg/deploysecrets/plan.go | 9 +--- pkg/environments/workspace.go | 4 +- pkg/internal/azureauth/credential.go | 4 +- pkg/solutionrun/frontend_build.go | 8 +++- pkg/solutionrun/frontend_local.go | 2 +- 7 files changed, 67 insertions(+), 51 deletions(-) diff --git a/cmd/environment/configuration_profile.go b/cmd/environment/configuration_profile.go index 23d01315d..e63a31854 100644 --- a/cmd/environment/configuration_profile.go +++ b/cmd/environment/configuration_profile.go @@ -3,6 +3,7 @@ package environment import ( "context" "fmt" + "io/fs" "os" "path/filepath" "regexp" @@ -55,14 +56,19 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env } } output := filepath.Join(destination, configurations.ProfileDerivationFile) - retired, err := retiredGeneratedConfigurations(destination, output) + confined, err := os.OpenRoot(configurationRoot) + if err != nil { + return err + } + defer confined.Close() + retired, err := retiredGeneratedConfigurations(confined, target) if err != nil { return err } if dryRun { return nil } - if err := os.MkdirAll(destination, 0o755); err != nil { + if err := confined.MkdirAll(target, 0o755); err != nil { return err } body := profileMarker + env.ConfigurationSource + "\nderives-from: " + env.ConfigurationSource + "\n" @@ -70,7 +76,7 @@ func materializeConfigurationProfile(ctx context.Context, directory string, env return err } for _, path := range retired { - if err := os.Remove(path); err != nil { + if err := confined.Remove(path); err != nil { return err } } @@ -89,7 +95,12 @@ func validateConfigurationSourceLayer(layer, destination string) error { if err != nil || resolved != layer { return fmt.Errorf("configuration source must be an ordinary profile directory") } - entries, err := os.ReadDir(layer) + confined, err := os.OpenRoot(layer) + if err != nil { + return err + } + defer confined.Close() + entries, err := fs.ReadDir(confined.FS(), ".") if err != nil { return err } @@ -108,7 +119,7 @@ func validateConfigurationSourceLayer(layer, destination string) error { return fmt.Errorf("configuration-source must not contain environment-specific runnable bindings") } if strings.HasSuffix(entry.Name(), ".secret.env") { - if err := validateSecretPlaceholders(filepath.Join(layer, entry.Name()), entry.Name()); err != nil { + if err := validateSecretPlaceholders(confined, entry.Name()); err != nil { return err } } @@ -118,8 +129,8 @@ func validateConfigurationSourceLayer(layer, destination string) error { // validateSecretPlaceholders refuses a source secret file whose keys carry a value: // a configuration source declares which secrets exist, never what they hold. -func validateSecretPlaceholders(path, name string) error { - body, err := os.ReadFile(path) +func validateSecretPlaceholders(confined *os.Root, name string) error { + body, err := confined.ReadFile(name) if err != nil { return err } @@ -139,13 +150,13 @@ func validateSecretPlaceholders(path, name string) error { // retiredGeneratedConfigurations lists the generated files an earlier import left in // the destination, which a re-import removes; it refuses to overwrite an authored one // and a destination that is not an ordinary directory. -func retiredGeneratedConfigurations(destination, output string) ([]string, error) { - if info, err := os.Lstat(destination); err == nil && !info.IsDir() { +func retiredGeneratedConfigurations(confined *os.Root, target string) ([]string, error) { + if info, err := confined.Lstat(target); err == nil && !info.IsDir() { return nil, fmt.Errorf("generated profile must be an ordinary directory") } else if err != nil && !os.IsNotExist(err) { return nil, err } - previous, err := os.ReadDir(destination) + previous, err := fs.ReadDir(confined.FS(), target) if err != nil && !os.IsNotExist(err) { return nil, err } @@ -154,12 +165,12 @@ func retiredGeneratedConfigurations(destination, output string) ([]string, error if !strings.HasSuffix(entry.Name(), ".env") && entry.Name() != configurations.ProfileDerivationFile { continue } - path := filepath.Join(destination, entry.Name()) + path := filepath.Join(target, entry.Name()) info, infoErr := entry.Info() if infoErr != nil || !info.Mode().IsRegular() { return nil, fmt.Errorf("generated configuration %s must be a regular file", entry.Name()) } - body, readErr := os.ReadFile(path) + body, readErr := confined.ReadFile(path) if readErr != nil { return nil, readErr } @@ -167,7 +178,7 @@ func retiredGeneratedConfigurations(destination, output string) ([]string, error if !owned && entry.Name() != "runnable-bindings.env" { return nil, fmt.Errorf("refusing to overwrite authored configuration %s", entry.Name()) } - if path != output && owned { + if entry.Name() != configurations.ProfileDerivationFile && owned { retired = append(retired, path) } } diff --git a/cmd/environment/import.go b/cmd/environment/import.go index 396b05cfb..cc90a5c24 100644 --- a/cmd/environment/import.go +++ b/cmd/environment/import.go @@ -208,29 +208,8 @@ func runImport(ctx context.Context, opts *importOptions) error { return err } declaredEnvironment := yamledit.MapValue(declaration, "environment") - // Product profiles are owned by the consuming workspace. A placement-only - // producer need not acquire product configuration to import its target. - if opts.configurationSource != "" { - if existing := contract.Environment.ConfigurationSource; existing != "" && existing != opts.configurationSource { - return fmt.Errorf("--configuration-source conflicts with the coordinate declaration") - } - contract.Environment.ConfigurationSource = opts.configurationSource - node, encodeErr := yamledit.Encode(opts.configurationSource) - if encodeErr != nil { - return encodeErr - } - yamledit.SetMapValue(declaredEnvironment, "configuration-source", node) - } - if len(opts.configurationProfiles) > 0 { - if contract.Environment.ConfigurationProfile != "" || len(contract.Environment.ConfigurationProfiles) > 0 { - return fmt.Errorf("--configuration-profiles cannot replace coordinate-declared profiles") - } - contract.Environment.ConfigurationProfiles = opts.configurationProfiles - node, encodeErr := yamledit.Encode(opts.configurationProfiles) - if encodeErr != nil { - return encodeErr - } - yamledit.SetMapValue(declaredEnvironment, "configuration-profiles", node) + if profileErr := applyConfigurationProfiles(opts, contract, declaredEnvironment); profileErr != nil { + return profileErr } file := filepath.Join(opts.dir, resources.WorkspaceConfigurationName) @@ -646,3 +625,32 @@ func init() { importCmd.Flags().StringVar(&importNamespace, "namespace", "", "Assert the namespace declared by the producer") importCmd.Flags().BoolVar(&importDryRun, "dry-run", false, "Print the unified diff of workspace.codefly.yaml and write nothing") } + +func applyConfigurationProfiles(opts *importOptions, contract *environments.CoordinateContract, declaredEnvironment *yaml.Node) error { + // Product profiles are owned by the consuming workspace. A placement-only + // producer need not acquire product configuration to import its target. + if opts.configurationSource != "" { + if existing := contract.Environment.ConfigurationSource; existing != "" && existing != opts.configurationSource { + return fmt.Errorf("--configuration-source conflicts with the coordinate declaration") + } + contract.Environment.ConfigurationSource = opts.configurationSource + node, encodeErr := yamledit.Encode(opts.configurationSource) + if encodeErr != nil { + return encodeErr + } + yamledit.SetMapValue(declaredEnvironment, "configuration-source", node) + } + if len(opts.configurationProfiles) > 0 { + if contract.Environment.ConfigurationProfile != "" || len(contract.Environment.ConfigurationProfiles) > 0 { + return fmt.Errorf("--configuration-profiles cannot replace coordinate-declared profiles") + } + contract.Environment.ConfigurationProfiles = opts.configurationProfiles + node, encodeErr := yamledit.Encode(opts.configurationProfiles) + if encodeErr != nil { + return encodeErr + } + yamledit.SetMapValue(declaredEnvironment, "configuration-profiles", node) + } + + return nil +} diff --git a/pkg/deploysecrets/plan.go b/pkg/deploysecrets/plan.go index cbd232b3e..8de52666d 100644 --- a/pkg/deploysecrets/plan.go +++ b/pkg/deploysecrets/plan.go @@ -608,14 +608,7 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, return rotatedProperty(state, property, declaredKey, generated[declaredKey]) } if state.known() { - if stored, present := state.document[property.Property]; present { - if len(holders) > 0 && stored != holderValue { - return PropertyPlan{}, "", fmt.Errorf("%s holds different values in %s and %s: one configuration value cannot be two; resolve it before planning", - strings.Join(property.Keys, ", "), state.secret.RemoteKey, holders[0]) - } - if err = refuseSuppliedConflict(property, isSupplied, suppliedValue, state.secret.RemoteKey+"#"+property.Property, stored); err != nil { - return PropertyPlan{}, "", err - } + if _, present := state.document[property.Property]; present { return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionKeep, Source: "stored"}, "", nil } } diff --git a/pkg/environments/workspace.go b/pkg/environments/workspace.go index d343be58a..7cae37bb7 100644 --- a/pkg/environments/workspace.go +++ b/pkg/environments/workspace.go @@ -205,8 +205,8 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, sele if env == nil { return fmt.Errorf("selected environment is nil") } - if err := env.Validate(); err != nil { - return err + if validationErr := env.Validate(); validationErr != nil { + return validationErr } } environments = selected diff --git a/pkg/internal/azureauth/credential.go b/pkg/internal/azureauth/credential.go index 21359e9b5..5bcddbe3e 100644 --- a/pkg/internal/azureauth/credential.go +++ b/pkg/internal/azureauth/credential.go @@ -19,7 +19,7 @@ func Credential(tenant string, lookup func(string) (string, bool)) (azcore.Token } id, err := uuid.Parse(tenant) if err != nil || id == uuid.Nil || id.String() != tenant { - return nil, errors.New("Azure publication requires a declared tenant UUID") + return nil, errors.New("publication requires a declared Azure tenant UUID") } mode, _ := lookup("CODEFLY_AZURE_CREDENTIAL") switch mode { @@ -35,7 +35,7 @@ func Credential(tenant string, lookup func(string) (string, bool)) (azcore.Token tokenFile, _ := lookup("AZURE_FEDERATED_TOKEN_FILE") clientID, err := uuid.Parse(client) if workloadTenant != tenant || err != nil || clientID == uuid.Nil || clientID.String() != client || !filepath.IsAbs(tokenFile) { - return nil, errors.New("Azure workload publication requires the declared tenant, a client UUID and an absolute federated token file") + return nil, errors.New("workload publication requires the declared Azure tenant, a client UUID and an absolute federated token file") } credential, err := azidentity.NewWorkloadIdentityCredential(&azidentity.WorkloadIdentityCredentialOptions{ TenantID: tenant, ClientID: client, TokenFilePath: tokenFile, diff --git a/pkg/solutionrun/frontend_build.go b/pkg/solutionrun/frontend_build.go index 11d6dc998..196567511 100644 --- a/pkg/solutionrun/frontend_build.go +++ b/pkg/solutionrun/frontend_build.go @@ -22,6 +22,8 @@ import ( // mirror serves it like any other base. const FrontendBuilderImage = "docker.io/library/node:24-bookworm-slim@sha256:a5f4366e834695d5f363b920c104953a535d3a9864d1bdf74566a0ebe01daee5" +const frontendBuildCommand = "build" + // FrontendProject is the npm project a component's frontend build runs in, // slash-separated and relative to the module: the remote's own directory // (fe-remote beside the services) when it holds a package.json, else the @@ -149,9 +151,11 @@ func buildFrontendBundle(ctx context.Context, module *resources.Module, destinat dockerfile := fmt.Sprintf("FROM %s AS build\nWORKDIR /source\nCOPY [\".\", \".\"]\nWORKDIR %s\nRUN [\"sh\", \"-c\", \"npm ci && npm run build\"]\nFROM scratch\nCOPY --from=build [\"%s/\", \"/\"]\n", FrontendBuilderImage, workdir, bundle) //nolint:gosec // G204: fixed docker executable, separate arguments, no shell; the Dockerfile arrives on stdin. - args := []string{"build"} + args := make([]string, 0, 10) if builder != "" { - args = []string{"buildx", "build", "--builder", builder} + args = append(args, "buildx", frontendBuildCommand, "--builder", builder) + } else { + args = append(args, frontendBuildCommand) } args = append(args, "--progress=plain", "--file", "-", "--output", "type=local,dest="+destination, root) //nolint:gosec // G204: fixed docker executable, separate arguments, no shell. diff --git a/pkg/solutionrun/frontend_local.go b/pkg/solutionrun/frontend_local.go index 6a3676add..554384a24 100644 --- a/pkg/solutionrun/frontend_local.go +++ b/pkg/solutionrun/frontend_local.go @@ -580,7 +580,7 @@ func (s *frontendSources) buildNatively(ctx context.Context, module *resources.M if _, err := os.Stat(filepath.Join(projectDir, "node_modules")); err != nil { install = true } - steps := [][]string{{"npm", "run", "build"}} + steps := [][]string{{"npm", "run", frontendBuildCommand}} if install { steps = append([][]string{{"npm", "ci"}}, steps...) } From 3b951bc9ef336f581c28e23f5b7745ee77295ce7 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:58:32 +0000 Subject: [PATCH 3/6] chore: collapse the generated command reference in review --- .gitattributes | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..344684db8 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +# Cobra owns this complete generated reference; authored command docs remain visible. +/docs/cli-reference.md linguist-generated=true From 3f550605bc6e43712d4d360e14510f05aa8ab040 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:58:33 +0000 Subject: [PATCH 4/6] fix(environment): resolve pinned modules before validating imports --- cmd/environment/import.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/environment/import.go b/cmd/environment/import.go index cc90a5c24..b5a628ef0 100644 --- a/cmd/environment/import.go +++ b/cmd/environment/import.go @@ -124,6 +124,9 @@ full delivery.intent.yaml is not interpreted.`, now: time.Now().UTC(), stdout: cmd.OutOrStdout(), } + if err := common.ResolvePinnedModules(ctx); err != nil { + return err + } if err := runImport(ctx, &opts); err != nil { return err } From 3d1de693ff283120b19fe8174a3c11bc275d70b1 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Wed, 28 Oct 2026 20:52:04 +0000 Subject: [PATCH 5/6] refactor: keep cloud-specific secret and signing code out of the CLI The CLI consumes infra-base outputs and stays cloud-neutral. Remove the Azure Key Vault secret store and release signer, the Azure credential helper, the Azure SDK, and the planner-input export/publisher image that existed only for them. Also drop changes nothing consumes: the `deploy gitops cell` verb, the --supplied/--mint planner flags, draft promotion PRs, and the Cloud KMS error-classification change. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GehdohoEWY15W4Vj1EBoox --- build/secrets-publisher/Dockerfile | 5 - cmd/deploy/gitops.go | 30 +-- cmd/deploy/secrets.go | 29 -- cmd/deploy/secrets_input.go | 131 --------- cmd/deploy/secrets_input_test.go | 72 ----- docs/cli-reference.md | 38 +-- docs/commands.md | 4 +- docs/deployment-secret-safety.md | 70 +---- docs/module-manifests.md | 29 -- go.mod | 10 - go.sum | 26 -- pkg/delivery/signing/azure_key_vault.go | 179 ------------- pkg/delivery/signing/azure_key_vault_test.go | 257 ------------------ pkg/delivery/signing/cloud_kms.go | 32 +-- pkg/delivery/signing/cloud_kms_test.go | 29 -- pkg/delivery/signing/signing.go | 2 +- pkg/delivery/signing/vault_transit.go | 13 +- pkg/deploysecrets/azure.go | 265 ------------------- pkg/deploysecrets/azure_test.go | 258 ------------------ pkg/deploysecrets/plan.go | 208 +++------------ pkg/deploysecrets/plan_test.go | 8 +- pkg/deploysecrets/retained_material_test.go | 84 ------ pkg/deploysecrets/rotate_test.go | 4 +- pkg/deploysecrets/store.go | 15 +- pkg/deploysecrets/supplied_test.go | 95 ------- pkg/gitops/cell.go | 25 +- pkg/gitops/cell_test.go | 24 -- pkg/gitops/publish.go | 1 - pkg/internal/azureauth/credential.go | 50 ---- pkg/internal/azureauth/credential_test.go | 47 ---- 30 files changed, 61 insertions(+), 1979 deletions(-) delete mode 100644 build/secrets-publisher/Dockerfile delete mode 100644 cmd/deploy/secrets_input.go delete mode 100644 cmd/deploy/secrets_input_test.go delete mode 100644 pkg/delivery/signing/azure_key_vault.go delete mode 100644 pkg/delivery/signing/azure_key_vault_test.go delete mode 100644 pkg/deploysecrets/azure.go delete mode 100644 pkg/deploysecrets/azure_test.go delete mode 100644 pkg/deploysecrets/retained_material_test.go delete mode 100644 pkg/deploysecrets/supplied_test.go delete mode 100644 pkg/internal/azureauth/credential.go delete mode 100644 pkg/internal/azureauth/credential_test.go diff --git a/build/secrets-publisher/Dockerfile b/build/secrets-publisher/Dockerfile deleted file mode 100644 index 3af6ab673..000000000 --- a/build/secrets-publisher/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -# Package the existing CLI for infrastructure-owned publisher Jobs. -# The operator builds codefly from the exact source revision before building this image. -FROM gcr.io/distroless/cc-debian12:nonroot@sha256:9dac0a79194e45a7da0158a9c6da57b217585af0786db3845d1f0ec1a0dd182f -COPY --chmod=0555 codefly /codefly -ENTRYPOINT ["/codefly"] diff --git a/cmd/deploy/gitops.go b/cmd/deploy/gitops.go index d652a8897..bb235a05b 100644 --- a/cmd/deploy/gitops.go +++ b/cmd/deploy/gitops.go @@ -34,32 +34,6 @@ var GitOpsCmd = &cobra.Command{ Short: "Render, publish, observe, and recover reviewed GitOps promotions", } -var gitOpsCellCmd = &cobra.Command{ - Use: "cell", - Short: "Derive the cell file from published module trees without rebuilding images", - Args: cobra.NoArgs, - RunE: func(_ *cobra.Command, _ []string) error { - ctx, done := common.NewContext() - defer done() - if err := common.ResolvePinnedModules(ctx); err != nil { - return err - } - workspace, err := common.LoadWorkspace(ctx) - if err != nil { - return err - } - env, err := orchestration.SelectEnvironment(workspace, gitOpsEnv) - if err != nil { - return err - } - cell, err := gitops.RenderPublishedCell(ctx, workspace, env) - if err == nil { - printCell(&cell) - } - return err - }, -} - var gitOpsRenderCmd = &cobra.Command{ Use: "render [module]", Short: "Render and validate a module-owned manifest tree", @@ -640,8 +614,8 @@ var ( ) func init() { - GitOpsCmd.AddCommand(gitOpsCellCmd, gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd) - for _, command := range []*cobra.Command{gitOpsCellCmd, gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd} { + GitOpsCmd.AddCommand(gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd) + for _, command := range []*cobra.Command{gitOpsSnapshotCmd, gitOpsRenderCmd, gitOpsPlanCmd, gitOpsPublishCmd, gitOpsObserveCmd, gitOpsRollbackCmd} { command.Flags().StringVar(&gitOpsEnv, "env", "local", "Environment to promote") } // The delivery verbs — the ones that render a manifest tree from the diff --git a/cmd/deploy/secrets.go b/cmd/deploy/secrets.go index 0ede8155a..2f204b9a4 100644 --- a/cmd/deploy/secrets.go +++ b/cmd/deploy/secrets.go @@ -27,10 +27,6 @@ var ( secretsAllowMissing bool secretsYes bool secretsModules []string - secretsMintKeys []string - secretsExportInput string - secretsInput string - secretsSupplied string secretsRotate []string ) @@ -60,11 +56,6 @@ not. Both are reported. Values are never printed: the plan names keys and sources only. ---supplied names a JSON object of secret key to value for the required ones: a -supplied value is written where nothing holds the key, never over a stored value -and never in place of a declared generator. An empty value is accepted only for -a key service-secrets.may-be-empty declares. - --dry-run prints the plan and writes nothing. --metadata-only additionally never reads a stored value: it knows which remote keys exist, not what they hold. @@ -87,12 +78,6 @@ restart its consumers. An identifier or signing key is never rotated.`, RunE: func(cmd *cobra.Command, _ []string) error { ctx, done := common.NewContext() defer done() - if secretsInput != "" { - return runSecretsInput(ctx, cmd) - } - if secretsExportInput != "" && (len(secretsModules) != 0 || len(secretsMintKeys) != 0 || secretsAllowMissing || secretsMetadataOnly || secretsSupplied != "") { - return fmt.Errorf("--export-input exports the full render without module, mint, allow-missing, metadata-only or supplied options") - } if secretsMetadataOnly && !secretsDryRun { return fmt.Errorf("--metadata-only plans without reading the store, so it cannot be applied: pass --dry-run") } @@ -120,13 +105,6 @@ restart its consumers. An identifier or signing key is never rotated.`, return fmt.Errorf("--module %s: not a module rendered for %s (rendered: %s)", module, env.Name, strings.Join(rendered.Modules, ", ")) } } - if secretsExportInput != "" { - return exportSecretsInput(secretsExportInput, env, rendered, workspaceServiceUniques(ctx, workspace, env)) - } - supplied, err := readSuppliedValues(secretsSupplied, rendered, env.ServiceSecrets.MayBeEmpty) - if err != nil { - return err - } groups := renderedByStore(rendered) rotate, err := rotationKeys(rendered, groups, secretsRotate) if err != nil { @@ -156,8 +134,6 @@ restart its consumers. An identifier or signing key is never rotated.`, ReadPayloads: !secretsMetadataOnly, Modules: secretsModules, MayBeEmpty: env.ServiceSecrets.MayBeEmpty, - MintKeys: secretsMintKeys, - Supplied: supplied, Configured: configured, Rotate: readBy(group, rotate), }) @@ -350,10 +326,5 @@ func init() { SecretsCmd.Flags().BoolVar(&secretsAllowMissing, "allow-missing", false, "Write what can be resolved even when some properties must still be supplied") SecretsCmd.Flags().BoolVarP(&secretsYes, "yes", "y", false, "Write without an interactive confirmation") SecretsCmd.Flags().StringSliceVar(&secretsModules, "module", nil, "Plan only these modules' remote keys (comma-separated or repeated)") - SecretsCmd.Flags().StringSliceVar(&secretsMintKeys, "mint", nil, "Acknowledge first creation of these signing secret keys (comma-separated or repeated); never rotate existing keys") - SecretsCmd.Flags().StringVar(&secretsExportInput, "export-input", "", "Export declarative planner inputs for execution by an infrastructure-owned publisher; reads no stored values") - SecretsCmd.Flags().StringVar(&secretsInput, "input", "", "Run exported planner inputs under an infrastructure-bound Azure workload identity") - SecretsCmd.Flags().StringVar(&secretsSupplied, "supplied", "", "JSON object of operator values by secret key, for keys nothing stores or generates; never printed") - SecretsCmd.MarkFlagsMutuallyExclusive("export-input", "input") SecretsCmd.Flags().StringSliceVar(&secretsRotate, "rotate", nil, "Replace a generated key's stored value and every stored value derived from it: //:, :, or the CODEFLY__ key (repeatable)") } diff --git a/cmd/deploy/secrets_input.go b/cmd/deploy/secrets_input.go deleted file mode 100644 index f5c55c014..000000000 --- a/cmd/deploy/secrets_input.go +++ /dev/null @@ -1,131 +0,0 @@ -package deploy - -import ( - "context" - "encoding/json" - "errors" - "io" - "os" - - "github.com/codefly-dev/cli/pkg/deploysecrets" - "github.com/codefly-dev/cli/pkg/environments" - "github.com/codefly-dev/cli/pkg/gitops" - "github.com/spf13/cobra" -) - -// Only declarations travel to the publisher. Existing values and generated -// material are resolved by the normal planner inside its authorized execution. -type secretsInputDocument struct { - Schema string - Environment string - Rendered gitops.RenderedEnvironment - Generators []environments.EnvironmentSecretGenerator - Services []string - MayBeEmpty []string -} - -const secretsInputSchema = "codefly.secrets-input/v1" - -func exportSecretsInput(path string, env *environments.Environment, rendered gitops.RenderedEnvironment, services []string) error { - document := secretsInputDocument{Schema: secretsInputSchema, Environment: env.Name, - Rendered: rendered, Generators: env.ServiceSecrets.Generate, Services: services, - MayBeEmpty: env.ServiceSecrets.MayBeEmpty} - file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) - if err != nil { - return errors.New("cannot exclusively create secret planner input file") - } - err = json.NewEncoder(file).Encode(document) - closeErr := file.Close() - return errors.Join(err, closeErr) -} - -func readSecretsInput(input io.Reader, storeName string) (secretsInputDocument, error) { - var document secretsInputDocument - decoder := json.NewDecoder(io.LimitReader(input, 1024*1024+1)) - decoder.DisallowUnknownFields() - if decoder.Decode(&document) != nil || decoder.Decode(new(any)) != io.EOF || - document.Schema != secretsInputSchema || document.Environment == "" || - storeName == "" || len(document.Rendered.Secrets) == 0 { - return document, errors.New("invalid secret planner input; contents withheld") - } - for _, secret := range document.Rendered.Secrets { - if secret.Store.Kind != "ClusterSecretStore" || secret.Store.Name != storeName { - return document, errors.New("planner input does not match the infrastructure-bound ClusterSecretStore") - } - } - declarations := environments.EnvironmentServiceSecrets{ - SecretStore: document.Rendered.Secrets[0].Store, Generate: document.Generators, - } - if declarations.Validate() != nil { - return document, errors.New("invalid secret generator declarations; contents withheld") - } - return document, nil -} - -func runSecretsInput(ctx context.Context, cmd *cobra.Command) error { - // The exported document cannot select its destination or impersonate a reader. - // Infrastructure binds these from the live store and its existing writer Job. - if os.Getenv("CODEFLY_AZURE_CREDENTIAL") != "workload" { - return errors.New("secret planner input execution requires Azure workload identity") - } - if secretsExportInput != "" || secretsMetadataOnly || len(secretsModules) != 0 || len(secretsMintKeys) != 0 || secretsAllowMissing { - return errors.New("secret planner input execution requires the complete input without mint or allow-missing") - } - file, err := os.Open(secretsInput) - if err != nil { - return errors.New("cannot open secret planner input") - } - defer file.Close() - info, err := file.Stat() - if err != nil || !info.Mode().IsRegular() || info.Size() > 1024*1024 { - return errors.New("secret planner input must be a regular file of at most 1 MiB") - } - document, err := readSecretsInput(file, os.Getenv("CODEFLY_SECRETS_STORE_NAME")) - if err != nil { - return err - } - supplied, err := readSuppliedValues(secretsSupplied, document.Rendered, document.MayBeEmpty) - if err != nil { - return err - } - store, err := deploysecrets.AzurePublisherStore(ctx, os.Getenv("CODEFLY_SECRETS_AZURE_VAULT_URL"), os.Getenv("AZURE_TENANT_ID")) - if err != nil { - return err - } - plan, err := deploysecrets.Build(ctx, &deploysecrets.Inputs{ - Rendered: document.Rendered, Generators: document.Generators, Services: document.Services, - MayBeEmpty: document.MayBeEmpty, Store: store, ReadPayloads: true, Supplied: supplied, - }) - if err != nil { - return err - } - printSecretsPlan(cmd.OutOrStdout(), document.Environment, document.Rendered, plan) - return finishSecretsPlan(ctx, plan, store, secretsDryRun, false, func() bool { return secretsYes }) -} - -// readSuppliedValues reads the operator's values for keys nothing stores or -// generates: one JSON object of secret key to string. Errors never quote the -// file, which holds credentials. -func readSuppliedValues(path string, rendered gitops.RenderedEnvironment, mayBeEmpty []string) (map[string]string, error) { - if path == "" { - return nil, nil - } - file, err := os.Open(path) - if err != nil { - return nil, errors.New("cannot open supplied values") - } - defer file.Close() - info, err := file.Stat() - if err != nil || !info.Mode().IsRegular() || info.Size() > 1024*1024 { - return nil, errors.New("supplied values must be a regular file of at most 1 MiB") - } - var supplied map[string]string - decoder := json.NewDecoder(io.LimitReader(file, 1024*1024+1)) - if decoder.Decode(&supplied) != nil || decoder.Decode(new(any)) != io.EOF || supplied == nil { - return nil, errors.New("supplied values must be one JSON object of secret key to string; contents withheld") - } - if err := deploysecrets.ValidateSupplied(rendered, supplied, mayBeEmpty); err != nil { - return nil, err - } - return supplied, nil -} diff --git a/cmd/deploy/secrets_input_test.go b/cmd/deploy/secrets_input_test.go deleted file mode 100644 index cca8b52f1..000000000 --- a/cmd/deploy/secrets_input_test.go +++ /dev/null @@ -1,72 +0,0 @@ -package deploy - -import ( - "bytes" - "encoding/json" - "os" - "strings" - "testing" - - "github.com/codefly-dev/cli/pkg/environments" - "github.com/codefly-dev/cli/pkg/gitops" -) - -func TestSecretsInputBindsEveryStoreAndRejectsPayloadFields(t *testing.T) { - document := secretsInputDocument{Schema: secretsInputSchema, Environment: "azure-staging", - Rendered: gitops.RenderedEnvironment{Secrets: []gitops.RenderedServiceSecret{ - {Store: environments.EnvironmentSecretStoreReference{Kind: "ClusterSecretStore", Name: "cell-secrets"}}, - }}} - raw, err := json.Marshal(document) - if err != nil { - t.Fatal(err) - } - if _, err := readSecretsInput(bytes.NewReader(raw), "cell-secrets"); err != nil { - t.Fatal(err) - } - for _, name := range []string{"", "another-store"} { - if _, err := readSecretsInput(bytes.NewReader(raw), name); err == nil { - t.Fatal("accepted wrong store") - } - } - for _, changed := range []string{ - strings.Replace(string(raw), `"Schema":`, `"Values":{"token":"never-print"},"Schema":`, 1), - string(raw) + `{}`, strings.Replace(string(raw), secretsInputSchema, "unknown/v2", 1), - strings.Replace(string(raw), "ClusterSecretStore", "SecretStore", 1), - } { - if _, err := readSecretsInput(strings.NewReader(changed), "cell-secrets"); err == nil || strings.Contains(err.Error(), "never-print") { - t.Fatal("invalid input was accepted or payload leaked") - } - } -} - -func TestSuppliedValuesAreOneObjectAndNeverQuoted(t *testing.T) { - key := "CODEFLY__WORKSPACE_SECRET_CONFIGURATION__IDENTITY__IDENTITY_CLIENT_SECRET" - rendered := gitops.RenderedEnvironment{Secrets: []gitops.RenderedServiceSecret{{ - RemoteKey: "host-accounts", - Properties: []gitops.RenderedSecretProperty{{Property: key, Keys: []string{key}}}, - }}} - path := t.TempDir() + "/supplied.json" - write := func(content string) { - t.Helper() - if err := os.WriteFile(path, []byte(content), 0o600); err != nil { - t.Fatal(err) - } - } - write(`{"` + key + `":"never-print"}`) - supplied, err := readSuppliedValues(path, rendered, nil) - if err != nil || supplied[key] != "never-print" { - t.Fatalf("readSuppliedValues = %v, %v", supplied, err) - } - for _, content := range []string{ - `["never-print"]`, `{"` + key + `":"never-print"}{}`, `{"` + key + `":1}`, `never-print`, - `{"CODEFLY__WORKSPACE_SECRET_CONFIGURATION__TYPO__X":"never-print"}`, - } { - write(content) - if _, err := readSuppliedValues(path, rendered, nil); err == nil || strings.Contains(err.Error(), "never-print") { - t.Fatalf("accepted %q or leaked it: %v", content, err) - } - } - if supplied, err := readSuppliedValues("", rendered, nil); err != nil || supplied != nil { - t.Fatalf("no file = %v, %v; want nothing", supplied, err) - } -} diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 91a762815..efc48c49d 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2125,7 +2125,6 @@ codefly deploy gitops Subcommands: -- [`codefly deploy gitops cell`](#codefly-deploy-gitops-cell) - [`codefly deploy gitops observe`](#codefly-deploy-gitops-observe) - [`codefly deploy gitops plan`](#codefly-deploy-gitops-plan) - [`codefly deploy gitops publish`](#codefly-deploy-gitops-publish) @@ -2134,20 +2133,6 @@ Subcommands: - [`codefly deploy gitops rollback`](#codefly-deploy-gitops-rollback) - [`codefly deploy gitops snapshot`](#codefly-deploy-gitops-snapshot) -## `codefly deploy gitops cell` - -Derive the cell file from published module trees without rebuilding images - -``` -codefly deploy gitops cell [flags] -``` - -Flags: - -``` - --env string Environment to promote (default "local") -``` - ## `codefly deploy gitops observe` Verify Argo CD reconciled the reviewed Git revision and store evidence @@ -2397,11 +2382,6 @@ not. Both are reported. Values are never printed: the plan names keys and sources only. ---supplied names a JSON object of secret key to value for the required ones: a -supplied value is written where nothing holds the key, never over a stored value -and never in place of a declared generator. An empty value is accepted only for -a key service-secrets.may-be-empty declares. - --dry-run prints the plan and writes nothing. --metadata-only additionally never reads a stored value: it knows which remote keys exist, not what they hold. @@ -2429,17 +2409,13 @@ codefly deploy secrets [flags] Flags: ``` - --allow-missing Write what can be resolved even when some properties must still be supplied - --dry-run Print the plan and write nothing - --env string Environment whose rendered ExternalSecrets to seed (default "local") - --export-input string Export declarative planner inputs for execution by an infrastructure-owned publisher; reads no stored values - --input string Run exported planner inputs under an infrastructure-bound Azure workload identity - --metadata-only With --dry-run: never read a stored value, only which remote keys exist - --mint strings Acknowledge first creation of these signing secret keys (comma-separated or repeated); never rotate existing keys - --module strings Plan only these modules' remote keys (comma-separated or repeated) - --rotate strings Replace a generated key's stored value and every stored value derived from it: //:, :, or the CODEFLY__ key (repeatable) - --supplied string JSON object of operator values by secret key, for keys nothing stores or generates; never printed - -y, --yes Write without an interactive confirmation + --allow-missing Write what can be resolved even when some properties must still be supplied + --dry-run Print the plan and write nothing + --env string Environment whose rendered ExternalSecrets to seed (default "local") + --metadata-only With --dry-run: never read a stored value, only which remote keys exist + --module strings Plan only these modules' remote keys (comma-separated or repeated) + --rotate strings Replace a generated key's stored value and every stored value derived from it: //:, :, or the CODEFLY__ key (repeatable) + -y, --yes Write without an interactive confirmation ``` ## `codefly deploy service` diff --git a/docs/commands.md b/docs/commands.md index e6930c76d..eead739b1 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2761,8 +2761,8 @@ resolved for blue rather than copied from a sibling. For a placement-only coordinate, the product may select its own source at import: ```bash -codefly environment import azure --coordinate-contract coordinate.json \ - --configuration-source hosted-azure --configuration-profiles azure,local +codefly environment import staging --coordinate-contract coordinate.json \ + --configuration-source hosted --configuration-profiles staging,local ``` These options use the same importer and profile inheritance. A source conflicting diff --git a/docs/deployment-secret-safety.md b/docs/deployment-secret-safety.md index e25519077..61419a78b 100644 --- a/docs/deployment-secret-safety.md +++ b/docs/deployment-secret-safety.md @@ -13,65 +13,8 @@ Backend stdout and stderr are withheld on failure, including debug formatting of backend errors. Backend diagnostics can contain payloads or HTTP response bodies; forwarding them is unsafe. The error names the command that failed and the planner adds the affected remote key. JSON decoding failures likewise omit the -backend content. GCP values travel on stdin and Azure values in the SDK's HTTPS -request body, never in process arguments. Rendering stores references, not secret -payloads. - -## Azure Key Vault - -The existing `azurekv` SecretStore or ClusterSecretStore selects the vault. -Publication supports Azure public-cloud WorkloadIdentity stores. The tenant comes -from the store's `tenantId` or its referenced service account's -`azure.workload.identity/tenant-id` annotation, exactly one source. A namespaced -store cannot reference a service account in another namespace. - -The Microsoft Key Vault SDK defaults to the operator's Azure CLI login, pinned to -that tenant. An authorized in-cluster publisher can explicitly select -`CODEFLY_AZURE_CREDENTIAL=workload`; the SDK then uses its projected -`AZURE_FEDERATED_TOKEN_FILE`, `AZURE_CLIENT_ID` and `AZURE_TENANT_ID`. The tenant must -match the store's declaration. Neither mode falls back to another credential, and -neither borrows the service account ESO uses to read secrets. The publisher must -already have its own write grant; this option creates no identity, grant or Job. -A mismatched vault tenant is refused by the SDK's authentication -challenge validation. Metadata-only planning lists active/deleted secret metadata -and versions; it never fetches a secret value. Disabled, expired, not-yet-valid, -managed and recoverable deleted secrets are refused, not treated as absent. -SDK errors omit response bodies and authentication diagnostics. - -An infrastructure-owned publisher can use `deploy secrets --export-input ` -to export the render's requirements and generator declarations, without reading -stored values or including credentials. `deploy secrets --input --yes` -runs the same planner inside the writer Job. Infrastructure binds the live -ClusterSecretStore name, vault URL and tenant through the Job environment; -the input cannot choose them. Execution requires workload identity and refuses -mixed stores, unknown input fields, explicit signing-key minting and partial -publication. `--dry-run` reports required values before any store write. - -Build or reuse: searched the existing CLI planner and Azure SDK adapter, infra-base's -vault seeder and runtime-credential bootstrap, and the handbook service inventory. -Reuse the planner's preservation and concurrency checks under the existing seeder -identity. The old seeder's document PUT loop cannot safely replace this planner; -no new secret backend, credential exchange or operator write grant is introduced. - -## Retained credentials and concurrent writers - -Existing keys with no readable version fail planning. First creation of an -`ed25519-pkcs8` signing key requires `--mint CODEFLY__…` naming its exact stored -configuration key; `--yes` alone is insufficient. An existing document missing -that signing property must have its original material restored. `--mint` does -not permit reminting it. Renaming an entire remote key still needs human review: -the current render cannot identify material filed under an old, unreferenced key. - -Apply checks all read sources against the plan before writing and checks each -destination again immediately before writing. Changed metadata or payloads abort -with the keys already written returned to the caller. Rebuild the plan after a -failure; existing values are retained on the retry. - -These are optimistic checks, not an atomic compare-and-set or an all-stores -transaction. GCP version creation and Azure Set Secret do not offer a conditional -version-creation operation here. Serialize writers to an environment; another -writer acting between the final check and write can still race. Runtime ESO -projection and grants require separate live qualification. +backend content. Values sent to the store travel on stdin, never in process +arguments. Rendering stores references, not secret payloads. ## Existing environments @@ -92,15 +35,6 @@ Configuration aliases must agree across every referenced property, including properties in one document. Aliases use one generated value when none exists, or reuse the consistent value already held anywhere in that alias group. -Required values the operator holds are passed with `--supplied `, a JSON -object of secret key to value, in either mode. The planner writes a supplied -value only where no remote key holds that configuration value. It refuses a -supplied value that differs from a stored one, one for a key a generator -declares, one for a key no rendered property reads, and an empty value for a -key the environment does not declare may-be-empty. Values never appear in the -plan, the output or an error. The in-cluster publisher reads the file from the -same transient Secret as its input. - An empty required property is an error. Missing values with no declared source are listed as required. Apply validates those requirements before confirmation or no-op success. `--dry-run` can report an incomplete plan without writing; diff --git a/docs/module-manifests.md b/docs/module-manifests.md index 9ec41a360..928fb7a83 100644 --- a/docs/module-manifests.md +++ b/docs/module-manifests.md @@ -533,35 +533,6 @@ KMS is selected when `CODEFLY_RELEASE_KMS_KEY` is set and no Transit variable is, and a publish that sets both is refused naming each, never one silently winning. -Azure Key Vault uses that same release-signing interface. Set -`CODEFLY_RELEASE_AZURE_KEY` to an exact version URL -(`https://.vault.azure.net/keys//<32-digit-version>` or -`https://.managedhsm.azure.net/keys//<32-digit-version>`) and -`CODEFLY_RELEASE_AZURE_TENANT` to the tenant UUID. The operator's Azure CLI -credential is restricted to that tenant; no fallback credential chain is used. -An authorized release publisher can instead explicitly select -`CODEFLY_AZURE_CREDENTIAL=workload`, with the projected `AZURE_FEDERATED_TOKEN_FILE`, -`AZURE_CLIENT_ID` and matching `AZURE_TENANT_ID`. This selects the publisher's own -workload identity; it neither grants signing access nor approves a host trust entry. -This adapter supports public-cloud Key Vault and Managed HSM endpoints and enabled, currently -valid EC or EC-HSM P-256 keys permitting `sign`. The operator needs public-key -read and signing permissions. Private key material remains in the selected custody. -The existing Azure SDK handles each endpoint's authentication challenge and resource audience. - -For Azure, `host.release.key_id` defaults to the **entire version URL**, so different -vaults cannot share a key ID accidentally. Core still owns the envelope and -public-key pin. A platform that declares logical release generations may set -`CODEFLY_RELEASE_AZURE_KEY_ID=delivery-release-v1`; this must exactly match the -physical key name in the version URL and have the `-v` shape. -The complete Azure version URL remains mandatory and pins every read/sign -operation. The host's reviewed public-key pin still determines which key is trusted; -the logical ID is not a substitute for that pin. The adapter verifies each returned ES256 signature before -converting Azure's fixed-width result to Core's ASN.1 format. Requests have a -30-second timeout, no automatic retries, and no redirects; errors withhold -backend response bodies. Configuring Azure together with Cloud KMS or Vault -Transit is refused. This is release signing only: runtime Host encryption, -HMAC and Ed25519 key operations require their own supported backend. - Developer custody is one Ed25519 key per machine, created or loaded by `codefly delivery key` (ID `dev-` plus 16 lowercase hex characters of the SHA-256 of its public key; the OS keychain where available, otherwise a 0600 diff --git a/go.mod b/go.mod index fd1ce0cd1..7e74a23d1 100644 --- a/go.mod +++ b/go.mod @@ -5,10 +5,6 @@ go 1.27.3 require ( connectrpc.com/connect v1.21.1 connectrpc.com/cors v0.1.0 - github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 - github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 - github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0 - github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0 github.com/Masterminds/semver v1.5.0 github.com/Masterminds/semver/v3 v3.5.0 github.com/Masterminds/sprig/v3 v3.3.0 @@ -71,10 +67,7 @@ require ( code.gitea.io/sdk/gitea v0.25.1 // indirect dario.cat/mergo v1.0.2 // indirect github.com/42wim/httpsig v1.2.4 // indirect - github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 // indirect - github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect - github.com/AzureAD/microsoft-authentication-library-for-go v1.7.0 // indirect github.com/Masterminds/goutils v1.1.1 // indirect github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29 // indirect github.com/ProtonMail/go-crypto v1.5.2 // indirect @@ -147,7 +140,6 @@ require ( github.com/go-openapi/swag/typeutils v0.29.2 // indirect github.com/go-openapi/swag/yamlutils v0.29.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect - github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect github.com/google/gnostic-models v0.7.1 // indirect github.com/google/go-github/v86 v86.0.0 // indirect @@ -164,7 +156,6 @@ require ( github.com/invopop/jsonschema v0.14.0 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/kevinburke/ssh_config v1.6.0 // indirect - github.com/kylelemons/godebug v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.4.1 // indirect github.com/lufia/plan9stats v0.0.0-20260802145828-341c2f0c90b5 // indirect github.com/mattn/go-colorable v0.1.15 // indirect @@ -188,7 +179,6 @@ require ( github.com/openai/openai-go v1.12.0 // indirect github.com/pb33f/ordered-map/v2 v2.3.1 // indirect github.com/pjbgf/sha1cd v0.7.0 // indirect - github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pkg/errors v0.9.1 // indirect github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect github.com/power-devops/perfstat v0.0.0-20260805114148-88456608a4f6 // indirect diff --git a/go.sum b/go.sum index 4b6f23670..5a622a262 100644 --- a/go.sum +++ b/go.sum @@ -18,26 +18,8 @@ dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8= dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA= github.com/42wim/httpsig v1.2.4 h1:mI5bH0nm4xn7K18fo1K3okNDRq8CCJ0KbBYWyA6r8lU= github.com/42wim/httpsig v1.2.4/go.mod h1:yKsYfSyTBEohkPik224QPFylmzEBtda/kjyIAJjh3ps= -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 h1:jHb/wfvRikGdxMXYV3QG/SzUOPYN9KEUUuC0Yd0/vC0= -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1/go.mod h1:pzBXCYn05zvYIrwLgtK8Ap8QcjRg+0i76tMQdWN6wOk= -github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 h1:Hk5QBxZQC1jb2Fwj6mpzme37xbCDdNTxU7O9eb5+LB4= -github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1/go.mod h1:IYus9qsFobWIc2YVwe/WPjcnyCkPKtnHAqUYeebc8z0= -github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2 h1:yz1bePFlP5Vws5+8ez6T3HWXPmwOK7Yvq8QxDBD3SKY= -github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2/go.mod h1:Pa9ZNPuoNu/GztvBSKk9J1cDJW6vk/n0zLtV4mgd8N8= -github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 h1:fhqpLE3UEXi9lPaBRpQ6XuRW0nU7hgg4zlmZZa+a9q4= -github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0/go.mod h1:7dCRMLwisfRH3dBupKeNCioWYUZ4SS09Z14H+7i8ZoY= -github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0 h1:MaKvxE6D0KkjOg6Wd9M00iqP5PR0kUxCfiezes4JweM= -github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0/go.mod h1:i2h9fsTFKZorh8RdV2IcSUf/Qj98GlTkrTvUbX/s8as= -github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0 h1:aMFOzch6ZJo4Ct9hI4A9Y2fPen5YNRTPmkSBhe5m0ZQ= -github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0/go.mod h1:Oct8bx+g+DXKngU7i/LzFzYt44rmLdMu4uoofIpooVo= -github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0 h1:nCYfgcSyHZXJI8J0IWE5MsCGlb2xp9fJiXyxWgmOFg4= -github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0/go.mod h1:ucUjca2JtSZboY8IoUqyQyuuXvwbMBVwFOm0vdQPNhA= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c h1:udKWzYgxTojEKWjV8V+WSxDXJ4NFATAsZjh8iIbsQIg= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= -github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJTmL004Abzc5wDB5VtZG2PJk5ndYDgVacGqfirKxjM= -github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= -github.com/AzureAD/microsoft-authentication-library-for-go v1.7.0 h1:4iB+IesclUXdP0ICgAabvq2FYLXrJWKx1fJQ+GxSo3Y= -github.com/AzureAD/microsoft-authentication-library-for-go v1.7.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk= github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI= github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= @@ -268,8 +250,6 @@ github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPE github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/gofrs/flock v0.13.1 h1:jjREztyBeSKBZYAC+mgc1laB+xsgy4kYMf3FbKF2UBo= github.com/gofrs/flock v0.13.1/go.mod h1:sf4BFiHwnvgxa25DlQoDqXQnwRMEOwqxRq37P6MzzmE= -github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= -github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 h1:f+oWsMOmNPc8JmEHVZIycC7hBoQxHH9pNKQORJNozsQ= github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8/go.mod h1:wcDNUvekVysuuOpQKo3191zZyTpiI6se1N1ULghS0sw= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= @@ -320,8 +300,6 @@ github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOl github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/kevinburke/ssh_config v1.6.0 h1:J1FBfmuVosPHf5GRdltRLhPJtJpTlMdKTBjRgTaQBFY= github.com/kevinburke/ssh_config v1.6.0/go.mod h1:q2RIzfka+BXARoNexmF9gkxEX7DmvbW9P4hIVx2Kg4M= -github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= -github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= @@ -329,8 +307,6 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= -github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lucasb-eyer/go-colorful v1.4.1 h1:1EO+WB73+EH8EVbzlrG3KLAfEypQWVHIBqlTf+2hNss= github.com/lucasb-eyer/go-colorful v1.4.1/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/lufia/plan9stats v0.0.0-20260802145828-341c2f0c90b5 h1:eveIIGn4BGM3qknO74omf6HYr30/exH+eVUTuAgwjZ0= @@ -397,8 +373,6 @@ github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdD github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pjbgf/sha1cd v0.7.0 h1:ZRNPKHj+gfkLBf0KJv/p1Hmz+7bqCT5o311YX0nq+DA= github.com/pjbgf/sha1cd v0.7.0/go.mod h1:pKR5Li+qTCo+ebqITZfwPlJGoCFCvSO00qxjbNtTUFQ= -github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= -github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= diff --git a/pkg/delivery/signing/azure_key_vault.go b/pkg/delivery/signing/azure_key_vault.go deleted file mode 100644 index 182f06af3..000000000 --- a/pkg/delivery/signing/azure_key_vault.go +++ /dev/null @@ -1,179 +0,0 @@ -package signing - -import ( - "context" - "crypto" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/sha256" - "encoding/asn1" - "errors" - "fmt" - "math/big" - "net/http" - "net/url" - "os" - "regexp" - "time" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys" - "github.com/codefly-dev/cli/pkg/internal/azureauth" - "github.com/google/uuid" -) - -var azureReleaseHost = regexp.MustCompile(`^[a-z][a-z0-9-]{1,22}[a-z0-9]\.(vault|managedhsm)\.azure\.net$`) -var azureReleasePath = regexp.MustCompile(`^/keys/([A-Za-z0-9-]{1,127})/([a-f0-9]{32})$`) -var azureLogicalReleaseID = regexp.MustCompile(`^[a-z0-9-]+-v[1-9][0-9]*$`) - -// AzureKeyVault signs with one explicitly versioned P-256 key. Only the public -// key leaves the vault; Core still owns delivery envelopes and admission. -type AzureKeyVault struct { - client *azkeys.Client - keyURI, name, version string - keyID string - public *ecdsa.PublicKey - publicBytes []byte -} - -// NewAzureKeyVault uses the explicitly selected tenant-bound publication credential. -func NewAzureKeyVault(lookup func(string) (string, bool)) (*AzureKeyVault, error) { - return newAzureKeyVault(lookup, nil, nil) -} - -func newAzureKeyVault(lookup func(string) (string, bool), credential azcore.TokenCredential, transport policy.Transporter) (*AzureKeyVault, error) { - if lookup == nil { - lookup = os.LookupEnv - } - keyURI, _ := lookup("CODEFLY_RELEASE_AZURE_KEY") - if keyURI == "" { - return nil, ErrNoIdentity - } - u, err := url.Parse(keyURI) - if err != nil || u.Scheme != "https" || !azureReleaseHost.MatchString(u.Host) || u.User != nil || - u.RawQuery != "" || u.ForceQuery || u.Fragment != "" || u.RawPath != "" { - return nil, errors.New("CODEFLY_RELEASE_AZURE_KEY must be an HTTPS public-cloud Azure Key Vault or Managed HSM key-version URL") - } - parts := azureReleasePath.FindStringSubmatch(u.Path) - if parts == nil { - return nil, errors.New("CODEFLY_RELEASE_AZURE_KEY requires an exact key name and 32-digit version") - } - keyID := keyURI - if declared, _ := lookup("CODEFLY_RELEASE_AZURE_KEY_ID"); declared != "" { - if !azureLogicalReleaseID.MatchString(declared) || declared != parts[1] { - return nil, errors.New("CODEFLY_RELEASE_AZURE_KEY_ID must match the versioned logical key name in CODEFLY_RELEASE_AZURE_KEY") - } - keyID = declared - } - if credential == nil { - tenant, _ := lookup("CODEFLY_RELEASE_AZURE_TENANT") - id, parseErr := uuid.Parse(tenant) - if parseErr != nil || id == uuid.Nil || id.String() != tenant { - return nil, errors.New("CODEFLY_RELEASE_AZURE_TENANT must name the declared tenant UUID") - } - credential, err = azureauth.Credential(tenant, lookup) - if err != nil { - return nil, err - } - } - if transport == nil { - transport = &http.Client{Timeout: 30 * time.Second, - CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} - } - client, err := azkeys.NewClient("https://"+u.Host, credential, &azkeys.ClientOptions{ - ClientOptions: azcore.ClientOptions{Transport: transport, Retry: policy.RetryOptions{MaxRetries: -1}}, - }) - if err != nil { - return nil, errors.New("cannot initialize Azure release custody") - } - signer := &AzureKeyVault{client: client, keyURI: keyURI, name: parts[1], version: parts[2], keyID: keyID} - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - response, err := client.GetKey(ctx, signer.name, signer.version, nil) - if err != nil { - return nil, azureSigningError(err) - } - signer.public, err = azureReleasePublicKey(keyURI, response.Key, response.Attributes) - if err != nil { - return nil, err - } - signer.publicBytes, err = signer.public.Bytes() - if err != nil { - return nil, errors.New("invalid Azure release public key encoding") - } - return signer, nil -} - -func azureReleasePublicKey(keyURI string, key *azkeys.JSONWebKey, attrs *azkeys.KeyAttributes) (*ecdsa.PublicKey, error) { - now := time.Now() - if attrs == nil || attrs.Enabled == nil || !*attrs.Enabled || - (attrs.NotBefore != nil && now.Before(*attrs.NotBefore)) || (attrs.Expires != nil && !now.Before(*attrs.Expires)) { - return nil, errors.New("the Azure release key is disabled or outside its validity period") - } - if key == nil || key.KID == nil || string(*key.KID) != keyURI || key.Kty == nil || - (*key.Kty != azkeys.KeyTypeEC && *key.Kty != azkeys.KeyTypeECHSM) || key.Crv == nil || *key.Crv != azkeys.CurveNameP256 || - len(key.X) != 32 || len(key.Y) != 32 || len(key.D) != 0 || len(key.K) != 0 { - return nil, errors.New("the Azure release custody requires the requested public P-256 key version") - } - canSign := false - for _, op := range key.KeyOps { - if op != nil && *op == azkeys.KeyOperationSign { - canSign = true - } - } - if !canSign { - return nil, errors.New("the Azure release key does not permit signing") - } - encoded := make([]byte, 65) - encoded[0] = 4 // SEC 1 uncompressed point. - copy(encoded[1:33], key.X) - copy(encoded[33:], key.Y) - public, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), encoded) - if err != nil { - return nil, errors.New("invalid Azure P-256 public key") - } - return public, nil -} - -func (s *AzureKeyVault) KeyID() string { return s.keyID } -func (s *AzureKeyVault) Public() crypto.PublicKey { - // This immutable encoding was validated during construction. - public, _ := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), s.publicBytes) - return public -} - -func (s *AzureKeyVault) Sign(message []byte) ([]byte, error) { - digest := sha256.Sum256(message) - algorithm := azkeys.SignatureAlgorithmES256 - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - response, err := s.client.Sign(ctx, s.name, s.version, azkeys.SignParameters{Algorithm: &algorithm, Value: digest[:]}, nil) - if err != nil { - return nil, azureSigningError(err) - } - if response.KID == nil || string(*response.KID) != s.keyURI || len(response.Result) != 64 { - return nil, errors.New("invalid Azure signature or a different key version") - } - // Azure ES256 returns fixed-width R || S; Core verifies ASN.1 DER. - r, scalarS := new(big.Int).SetBytes(response.Result[:32]), new(big.Int).SetBytes(response.Result[32:]) - if !ecdsa.Verify(s.public, digest[:], r, scalarS) { - return nil, ErrSignature - } - return asn1.Marshal(struct{ R, S *big.Int }{r, scalarS}) -} - -func azureSigningError(err error) error { - var response *azcore.ResponseError - if errors.As(err, &response) { - return fmt.Errorf("the Azure release custody request failed (HTTP %d; backend output withheld)", response.StatusCode) - } - return errors.New("the Azure release custody authentication or transport failed (backend output withheld)") -} - -func azureCustodyConfigured(lookup func(string) (string, bool)) bool { - key, _ := lookup("CODEFLY_RELEASE_AZURE_KEY") - tenant, _ := lookup("CODEFLY_RELEASE_AZURE_TENANT") - logicalID, _ := lookup("CODEFLY_RELEASE_AZURE_KEY_ID") - return key != "" || tenant != "" || logicalID != "" -} diff --git a/pkg/delivery/signing/azure_key_vault_test.go b/pkg/delivery/signing/azure_key_vault_test.go deleted file mode 100644 index 8fcd8c027..000000000 --- a/pkg/delivery/signing/azure_key_vault_test.go +++ /dev/null @@ -1,257 +0,0 @@ -package signing - -import ( - "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "encoding/base64" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "net/url" - "strings" - "testing" - "time" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - "github.com/codefly-dev/core/solutionhost" - "github.com/stretchr/testify/require" -) - -const azureTestKey = "https://release.vault.azure.net/keys/delivery/0123456789abcdef0123456789abcdef" - -type azureReleaseCredential struct{ resource string } - -func (credential azureReleaseCredential) GetToken(_ context.Context, options policy.TokenRequestOptions) (azcore.AccessToken, error) { - resource := credential.resource - if resource == "" { - resource = "https://vault.azure.net" - } - if len(options.Scopes) != 1 || options.Scopes[0] != resource+"/.default" { - return azcore.AccessToken{}, fmt.Errorf("unexpected scope") - } - return azcore.AccessToken{Token: "private-release-test-token", ExpiresOn: time.Now().Add(time.Hour)}, nil -} - -type azureReleaseTransport func(*http.Request) (*http.Response, error) - -func (f azureReleaseTransport) Do(r *http.Request) (*http.Response, error) { return f(r) } - -func azureReleaseLookup(values map[string]string) func(string) (string, bool) { - return func(key string) (string, bool) { value, ok := values[key]; return value, ok } -} - -func azureReleaseFixture(t *testing.T, mutate func(string, map[string]any), failSign bool) (*AzureKeyVault, error, *int) { - t.Helper() - return azureReleaseFixtureForKey(t, azureTestKey, "https://vault.azure.net", "EC", mutate, failSign) -} - -func azureReleaseFixtureForKey(t *testing.T, keyURI, resource, keyType string, mutate func(string, map[string]any), failSign bool, logicalID ...string) (*AzureKeyVault, error, *int) { - t.Helper() - requested, err := url.Parse(keyURI) - require.NoError(t, err) - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - calls := 0 - server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get("Authorization") == "" { - w.Header().Set("WWW-Authenticate", `Bearer authorization="https://login.microsoftonline.com/tenant", resource="`+resource+`"`) - w.WriteHeader(http.StatusUnauthorized) - return - } - require.Equal(t, "Bearer private-release-test-token", r.Header.Get("Authorization")) - calls++ - w.Header().Set("Content-Type", "application/json") - var response map[string]any - switch r.Method { - case http.MethodGet: - require.Equal(t, requested.Path, r.URL.Path) - response = map[string]any{ - "key": map[string]any{"kid": keyURI, "kty": keyType, "crv": "P-256", "key_ops": []string{"sign", "verify"}, - "x": base64.RawURLEncoding.EncodeToString(key.X.FillBytes(make([]byte, 32))), - "y": base64.RawURLEncoding.EncodeToString(key.Y.FillBytes(make([]byte, 32)))}, - "attributes": map[string]any{"enabled": true}, - } - case http.MethodPost: - require.Equal(t, requested.Path+"/sign", r.URL.Path) - if failSign { - w.WriteHeader(http.StatusForbidden) - fmt.Fprint(w, `{"error":{"message":"private-release-test-token"}}`) - return - } - var request struct { - Algorithm string `json:"alg"` - Value string `json:"value"` - } - require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) - require.Equal(t, "ES256", request.Algorithm) - digest, err := base64.RawURLEncoding.DecodeString(request.Value) - require.NoError(t, err) - require.Len(t, digest, 32) - r, s, err := ecdsa.Sign(rand.Reader, key, digest) - require.NoError(t, err) - raw := append(r.FillBytes(make([]byte, 32)), s.FillBytes(make([]byte, 32))...) - response = map[string]any{"kid": keyURI, "value": base64.RawURLEncoding.EncodeToString(raw)} - default: - t.Errorf("unexpected operation %s", r.Method) - } - if mutate != nil { - mutate(r.Method, response) - } - require.NoError(t, json.NewEncoder(w).Encode(response)) - })) - t.Cleanup(server.Close) - u, err := url.Parse(server.URL) - require.NoError(t, err) - client := server.Client() - client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } - transport := azureReleaseTransport(func(r *http.Request) (*http.Response, error) { - local := r.Clone(r.Context()) - local.Host, local.URL.Host = r.URL.Host, u.Host - response, err := client.Do(local) - if response != nil { - response.Request = r - } - return response, err - }) - values := map[string]string{"CODEFLY_RELEASE_AZURE_KEY": keyURI} - if len(logicalID) > 0 { - values["CODEFLY_RELEASE_AZURE_KEY_ID"] = logicalID[0] - } - signer, err := newAzureKeyVault(azureReleaseLookup(values), azureReleaseCredential{resource: resource}, transport) - return signer, err, &calls -} - -func TestAzureReleaseLogicalIDKeepsExactRemoteVersionAndCoreAdmission(t *testing.T) { - keyURI := strings.Replace(azureTestKey, "/delivery/", "/delivery-release-v1/", 1) - signer, err, calls := azureReleaseFixtureForKey(t, keyURI, "https://vault.azure.net", "EC", nil, false, "delivery-release-v1") - require.NoError(t, err) - check, err := KeyCheck(signer, solutionhost.KeyKindRelease, "acme", "delivery-release-v1") - require.NoError(t, err) - payload := testPayload(t) - envelope, err := SignEnvelope(payload, signer) - require.NoError(t, err) - require.NoError(t, check(context.Background(), envelope, payload)) - require.Equal(t, 2, *calls) - for _, mismatched := range []string{"delivery-release-v2", "another-key-v1", "delivery-release"} { - _, err, calls = azureReleaseFixtureForKey(t, keyURI, "https://vault.azure.net", "EC", nil, false, mismatched) - require.Error(t, err) - require.Zero(t, *calls) - } -} - -func TestAzureManagedHSMReleaseSignsTheCoreDeliveryEnvelope(t *testing.T) { - keyURI := strings.Replace(azureTestKey, ".vault.", ".managedhsm.", 1) - signer, err, calls := azureReleaseFixtureForKey(t, keyURI, "https://managedhsm.azure.net", "EC-HSM", nil, false) - require.NoError(t, err) - check, err := KeyCheck(signer, solutionhost.KeyKindRelease, "acme", keyURI) - require.NoError(t, err) - payload := testPayload(t) - envelope, err := SignEnvelope(payload, signer) - require.NoError(t, err) - require.NoError(t, check(context.Background(), envelope, payload)) - require.Equal(t, 2, *calls) -} - -func TestAzureManagedHSMRefusesAnotherResourceChallenge(t *testing.T) { - keyURI := strings.Replace(azureTestKey, ".vault.", ".managedhsm.", 1) - _, err, calls := azureReleaseFixtureForKey(t, keyURI, "https://vault.azure.net", "EC-HSM", nil, false) - require.Error(t, err) - require.Zero(t, *calls) -} - -func TestAzureReleaseSignsTheCoreDeliveryEnvelope(t *testing.T) { - signer, err, calls := azureReleaseFixture(t, nil, false) - require.NoError(t, err) - check, err := KeyCheck(signer, solutionhost.KeyKindRelease, "acme", azureTestKey) - require.NoError(t, err) - payload := testPayload(t) - envelope, err := SignEnvelope(payload, signer) - require.NoError(t, err) - require.NoError(t, check(context.Background(), envelope, payload)) - require.Equal(t, 2, *calls) - _, err = KeyCheck(signer, solutionhost.KeyKindRelease, "acme", "another-version") - require.Error(t, err) - wrongDomain, err := KeyCheck(signer, solutionhost.KeyKindRelease, "other", azureTestKey) - require.NoError(t, err) - require.Error(t, wrongDomain(context.Background(), envelope, payload)) - // A caller cannot mutate the key the custody later verifies against. - signer.Public().(*ecdsa.PublicKey).X.SetInt64(0) - _, err = signer.Sign([]byte("next document")) - require.NoError(t, err) -} - -func TestAzureReleaseRefusesInvalidKeys(t *testing.T) { - for _, name := range []string{"version", "curve", "disabled", "expired", "not-yet-valid", "operations", "point", "private"} { - t.Run(name, func(t *testing.T) { - _, err, _ := azureReleaseFixture(t, func(_ string, body map[string]any) { - key, attrs := body["key"].(map[string]any), body["attributes"].(map[string]any) - switch name { - case "version": - key["kid"] = azureTestKey + "other" - case "curve": - key["crv"] = "P-384" - case "disabled": - attrs["enabled"] = false - case "expired": - attrs["exp"] = time.Now().Add(-time.Hour).Unix() - case "not-yet-valid": - attrs["nbf"] = time.Now().Add(time.Hour).Unix() - case "operations": - key["key_ops"] = []string{"verify"} - case "point": - key["x"] = base64.RawURLEncoding.EncodeToString(make([]byte, 32)) - case "private": - key["d"] = "AQ" - } - }, false) - require.Error(t, err) - }) - } -} - -func TestAzureReleaseRefusesWrongSignaturesAndRedactsFailures(t *testing.T) { - for _, mode := range []string{"version", "invalid", "short", "denied"} { - t.Run(mode, func(t *testing.T) { - signer, err, calls := azureReleaseFixture(t, func(method string, body map[string]any) { - if method != http.MethodPost { - return - } - switch mode { - case "version": - body["kid"] = azureTestKey + "other" - case "invalid": - body["value"] = base64.RawURLEncoding.EncodeToString(make([]byte, 64)) - case "short": - body["value"] = "AQ" - } - }, mode == "denied") - require.NoError(t, err) - _, err = signer.Sign([]byte("document")) - require.Error(t, err) - require.NotContains(t, err.Error(), "private-release-test-token") - require.Equal(t, 2, *calls, "failed signing must not retry") - }) - } -} - -func TestAzureReleaseRefusesUnsafeConfigurationBeforeNetwork(t *testing.T) { - for _, key := range []string{ - strings.Replace(azureTestKey, "https:", "http:", 1), strings.Replace(azureTestKey, "vault.azure.net", "evil.example", 1), - azureTestKey + "?query=1", azureTestKey + "?", azureTestKey + "#fragment", azureTestKey + "/extra", - "https://release.vault.azure.net/keys/delivery", strings.Replace(azureTestKey, "/keys/", "/%6beys/", 1), - strings.Replace(azureTestKey, "vault.azure.net", "managedhsm.azure.net.evil.example", 1), - } { - _, err := NewAzureKeyVault(azureReleaseLookup(map[string]string{"CODEFLY_RELEASE_AZURE_KEY": key})) - require.Error(t, err) - } - _, err := NewAzureKeyVault(azureReleaseLookup(map[string]string{"CODEFLY_RELEASE_AZURE_KEY": azureTestKey})) - require.ErrorContains(t, err, "TENANT") - for _, other := range []string{"CODEFLY_RELEASE_KMS_KEY", "VAULT_TOKEN"} { - _, err := newReleaseSigner(azureReleaseLookup(map[string]string{"CODEFLY_RELEASE_AZURE_KEY": azureTestKey, other: "set"})) - require.ErrorContains(t, err, "two release custodies") - } -} diff --git a/pkg/delivery/signing/cloud_kms.go b/pkg/delivery/signing/cloud_kms.go index 80675eced..6e92ab6d8 100644 --- a/pkg/delivery/signing/cloud_kms.go +++ b/pkg/delivery/signing/cloud_kms.go @@ -44,7 +44,6 @@ const kmsScope = "https://www.googleapis.com/auth/cloudkms" const kmsVersionShape = "projects//locations//keyRings//cryptoKeys//cryptoKeyVersions/" var kmsEnumName = regexp.MustCompile(`^[A-Z0-9_]{1,64}$`) -var googleCredentialStatus = regexp.MustCompile(`^oauth2/google(?:/externalaccount)?: status code ([1-5][0-9]{2}):`) // CloudKMS is the optional adapter the handbook decision admits behind the // same custody interface as Vault Transit: public metadata and an @@ -210,14 +209,7 @@ func (s *CloudKMS) request(method, operation string, body []byte, result any) er if err != nil { var exchange *oauth2.RetrieveError if errors.As(err, &exchange) && exchange.Response != nil { - return kmsCredentialRefusal(strconv.Itoa(exchange.Response.StatusCode), string(exchange.Body)) - } - // Google's impersonation/STS token sources return an untyped error - // containing the response body. Retain only its known status prefix. - for cause := err; cause != nil; cause = errors.Unwrap(cause) { - if status := googleCredentialStatus.FindStringSubmatch(cause.Error()); status != nil { - return kmsCredentialRefusal(status[1], strings.TrimPrefix(cause.Error(), status[0])) - } + return fmt.Errorf("the Google credential was not exchanged for a Cloud KMS token (HTTP %d)", exchange.Response.StatusCode) } return fmt.Errorf("the Cloud KMS %s request failed", stage) } @@ -231,28 +223,6 @@ func (s *CloudKMS) request(method, operation string, body []byte, result any) er return nil } -// Only named Google error reasons may leave a credential response; messages and -// metadata can echo credentials or request bodies. -func kmsCredentialRefusal(status, body string) error { - message := fmt.Sprintf("the Google credential was not exchanged for a Cloud KMS token (HTTP %s)", status) - var response struct { - Error struct { - Details []struct { - Reason string `json:"reason"` - } `json:"details"` - } `json:"error"` - } - if len(body) <= 65536 && json.Unmarshal([]byte(body), &response) == nil { - for _, detail := range response.Error.Details { - switch detail.Reason { - case "ACCESS_TOKEN_SCOPE_INSUFFICIENT", "IAM_PERMISSION_DENIED", "SERVICE_DISABLED", "SECURITY_POLICY_VIOLATED": - return fmt.Errorf("%s: %s", message, detail.Reason) - } - } - } - return errors.New(message) -} - // kmsHost admits only Google's API hosts, where the ambient credential's // bearer token belongs, and loopback for a fake. func kmsHost(address string) bool { diff --git a/pkg/delivery/signing/cloud_kms_test.go b/pkg/delivery/signing/cloud_kms_test.go index cab905ec1..d1f03d6df 100644 --- a/pkg/delivery/signing/cloud_kms_test.go +++ b/pkg/delivery/signing/cloud_kms_test.go @@ -11,7 +11,6 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" - "errors" "fmt" "io" "net/http" @@ -258,34 +257,6 @@ func TestCloudKMSRefusalsNeverExposeToken(t *testing.T) { } } -type failedKMSCredential struct{ err error } - -func (c failedKMSCredential) Token() (*oauth2.Token, error) { return nil, c.err } - -func TestCloudKMSCredentialRefusalsKeepStatusWithoutResponseBody(t *testing.T) { - for _, prefix := range []string{"oauth2/google", "oauth2/google/externalaccount"} { - credential := failedKMSCredential{errors.New(prefix + ": status code 403: private-test-token")} - _, err := NewCloudKMS(kmsLookup("http://localhost"), credential) - if err == nil || !strings.Contains(err.Error(), "HTTP 403") || strings.Contains(err.Error(), "private-test-token") { - t.Fatalf("unsafe or unhelpful credential refusal: %v", err) - } - } -} - -func TestCloudKMSCredentialReasonsAreAllowlisted(t *testing.T) { - for _, reason := range []string{"ACCESS_TOKEN_SCOPE_INSUFFICIENT", "IAM_PERMISSION_DENIED", "private-test-token"} { - body := `{"error":{"message":"private-test-token","details":[{"reason":"` + reason + `"}]}}` - credential := failedKMSCredential{errors.New("oauth2/google: status code 403: " + body)} - _, err := NewCloudKMS(kmsLookup("http://localhost"), credential) - if err == nil || strings.Contains(err.Error(), "private-test-token") || !strings.Contains(err.Error(), "HTTP 403") { - t.Fatalf("unsafe or unhelpful refusal: %v", err) - } - if reason != "private-test-token" && !strings.Contains(err.Error(), reason) { - t.Fatalf("known reason omitted: %v", err) - } - } -} - func TestCloudKMSRejectsUnsafeConfiguration(t *testing.T) { for _, endpoint := range []string{"http://example.test", "http://192.0.2.1", "ftp://localhost", "https://private-test-token@example.test", "https://example.test?token=private-test-token", // The bearer token belongs to Google's hosts only. diff --git a/pkg/delivery/signing/signing.go b/pkg/delivery/signing/signing.go index e18f0f6f5..64b76ad8f 100644 --- a/pkg/delivery/signing/signing.go +++ b/pkg/delivery/signing/signing.go @@ -15,7 +15,7 @@ import ( // Signer is core's custody boundary. Core owns envelope construction and verification. type Signer = solutionhost.Signer -var ErrNoIdentity = errors.New("no delivery signing key: release publishing requires VAULT_ADDR, VAULT_TOKEN and CODEFLY_RELEASE_TRANSIT_KEY (Vault Transit custody), CODEFLY_RELEASE_KMS_KEY (Cloud KMS custody), or CODEFLY_RELEASE_AZURE_KEY and CODEFLY_RELEASE_AZURE_TENANT (Azure Key Vault custody)") +var ErrNoIdentity = errors.New("no delivery signing key: release publishing requires VAULT_ADDR, VAULT_TOKEN and CODEFLY_RELEASE_TRANSIT_KEY (Vault Transit custody) or CODEFLY_RELEASE_KMS_KEY (Cloud KMS custody)") var ErrSignature = errors.New("delivery document signature verification failed") // Unavailable records why this process cannot sign. diff --git a/pkg/delivery/signing/vault_transit.go b/pkg/delivery/signing/vault_transit.go index 56e812159..f5ee31eea 100644 --- a/pkg/delivery/signing/vault_transit.go +++ b/pkg/delivery/signing/vault_transit.go @@ -48,8 +48,10 @@ func FromEnvironment(lookup func(string) (string, bool)) Signer { return signer } -// newReleaseSigner selects exactly one configured release custody: Azure Key -// Vault, Google Cloud KMS, or Vault Transit. Conflicting selections are refused. +// newReleaseSigner selects the one custody the environment configures: Cloud +// KMS when CODEFLY_RELEASE_KMS_KEY is set and no Transit variable is, Vault +// Transit otherwise. Both configured is refused, naming each, rather than +// one silently winning. func newReleaseSigner(lookup func(string) (string, bool)) (Signer, error) { if lookup == nil { lookup = os.LookupEnv @@ -60,13 +62,6 @@ func newReleaseSigner(lookup func(string) (string, bool)) (Signer, error) { transit = append(transit, name) } } - if azureCustodyConfigured(lookup) { - kms, _ := lookup("CODEFLY_RELEASE_KMS_KEY") - if kms != "" || len(transit) > 0 { - return nil, errors.New("two release custodies are configured: Azure Key Vault and Cloud KMS or Vault Transit; unset one") - } - return NewAzureKeyVault(lookup) - } if kms, _ := lookup("CODEFLY_RELEASE_KMS_KEY"); kms != "" { if len(transit) > 0 { return nil, fmt.Errorf("two release custodies are configured: Cloud KMS (CODEFLY_RELEASE_KMS_KEY) and Vault Transit (%s); unset one", strings.Join(transit, ", ")) diff --git a/pkg/deploysecrets/azure.go b/pkg/deploysecrets/azure.go deleted file mode 100644 index 722b6414f..000000000 --- a/pkg/deploysecrets/azure.go +++ /dev/null @@ -1,265 +0,0 @@ -package deploysecrets - -import ( - "context" - "crypto/sha256" - "encoding/json" - "errors" - "fmt" - "net/url" - "regexp" - "slices" - "strings" - "time" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" - "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" - "github.com/codefly-dev/cli/pkg/environments" - "github.com/codefly-dev/cli/pkg/internal/azureauth" -) - -// AzureKeyVault uses the selected publisher identity, restricted to the ESO -// store's tenant. It never borrows ESO's credential or falls back to another -// credential source. The SDK owns authentication and paging. -type AzureKeyVault struct { - vaultURL string - client *azsecrets.Client -} - -var azureSecretName = regexp.MustCompile(`^[A-Za-z0-9-]{1,127}$`) -var azureVaultHost = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]{1,22}[a-zA-Z0-9]\.vault\.azure\.net$`) - -// AzurePublisherStore reuses the live-store adapter inside an infrastructure-owned -// publisher Job. Infrastructure supplies the destination, never the input document. -func AzurePublisherStore(ctx context.Context, vaultURL, tenantID string) (Store, error) { - raw, err := json.Marshal(map[string]string{"vaultUrl": vaultURL, "tenantId": tenantID, "authType": "WorkloadIdentity"}) - if err != nil { - return nil, errors.New("invalid publisher store binding") - } - return resolveAzureStore(ctx, nil, ClusterTarget{}, environments.EnvironmentSecretStoreReference{}, "", raw) -} - -func resolveAzureStore(ctx context.Context, run Runner, target ClusterTarget, ref environments.EnvironmentSecretStoreReference, namespace string, raw json.RawMessage) (Store, error) { - var provider struct { - VaultURL string `json:"vaultUrl"` - TenantID string `json:"tenantId"` - AuthType string `json:"authType"` - EnvironmentType string `json:"environmentType"` - ServiceAccountRef struct{ Name, Namespace string } `json:"serviceAccountRef"` - } - if json.Unmarshal(raw, &provider) != nil { - return nil, errors.New("invalid azurekv provider configuration (backend output withheld)") - } - u, parseErr := url.Parse(provider.VaultURL) - if parseErr != nil || u.Scheme != "https" || !azureVaultHost.MatchString(u.Host) || u.User != nil || - (u.Path != "" && u.Path != "/") || u.RawQuery != "" || u.Fragment != "" { - return nil, errors.New("azurekv requires an HTTPS Azure public-cloud vault URL") - } - if provider.AuthType != "WorkloadIdentity" || (provider.EnvironmentType != "" && provider.EnvironmentType != "PublicCloud") { - return nil, errors.New("azurekv publication currently supports PublicCloud WorkloadIdentity stores") - } - if provider.ServiceAccountRef.Name != "" { - saNamespace := provider.ServiceAccountRef.Namespace - if ref.Kind == "SecretStore" { - if saNamespace != "" && saNamespace != namespace { - return nil, errors.New("azurekv SecretStore service account must be in the store namespace") - } - saNamespace = namespace - } - if saNamespace == "" { - return nil, errors.New("azurekv ClusterSecretStore service account requires a namespace") - } - output, err := run(ctx, nil, "kubectl", "get", "serviceaccount", provider.ServiceAccountRef.Name, - "--namespace", saNamespace, "--kubeconfig", target.Kubeconfig, "--context", target.Context, "-o", "json") - if err != nil { - return nil, err - } - var account struct { - Metadata struct{ Annotations map[string]string } - } - if json.Unmarshal(output, &account) != nil { - return nil, errors.New("invalid azurekv service account metadata (backend output withheld)") - } - tenant := account.Metadata.Annotations["azure.workload.identity/tenant-id"] - if provider.TenantID != "" && tenant != "" { - return nil, errors.New("azurekv tenant is declared twice; use the same single tenant source as ESO") - } - if tenant != "" { - provider.TenantID = tenant - } - } - if provider.TenantID == "" { - return nil, errors.New("azurekv declares no tenant identity") - } - credential, err := azureauth.Credential(provider.TenantID, nil) - if err != nil { - return nil, err - } - client, err := azsecrets.NewClient(strings.TrimRight(provider.VaultURL, "/"), credential, nil) - if err != nil { - return nil, errors.New("cannot initialize Azure Key Vault client") - } - return &AzureKeyVault{vaultURL: strings.TrimRight(provider.VaultURL, "/"), client: client}, nil -} - -func (store *AzureKeyVault) Name() string { return "azurekv " + store.vaultURL } - -func (store *AzureKeyVault) Describe(ctx context.Context, key string) (Description, error) { - if !azureSecretName.MatchString(key) { - return Description{}, errors.New("invalid Azure Key Vault secret name") - } - pager := store.client.NewListSecretPropertiesPager(nil) - for pager.More() { - page, err := pager.NextPage(ctx) - if err != nil { - return Description{}, azureError(err) - } - if page.Value == nil { - return Description{}, errors.New("incomplete Azure secret listing") - } - for _, property := range page.Value { - if property == nil || property.ID == nil { - return Description{}, errors.New("incomplete Azure secret metadata") - } - if !strings.EqualFold(property.ID.Name(), key) { - continue - } - attributes := property.Attributes - if attributes == nil || attributes.Enabled == nil { - return Description{}, errors.New("incomplete Azure secret attributes") - } - now := time.Now() - if !*attributes.Enabled || (attributes.Expires != nil && !now.Before(*attributes.Expires)) || - (attributes.NotBefore != nil && now.Before(*attributes.NotBefore)) || (property.Managed != nil && *property.Managed) { - return Description{}, errors.New("azure secret is disabled, expired, not yet valid or externally managed; refusing to replace it") - } - // Include every version's metadata: listing base properties alone can - // miss two new versions created within the same timestamp second. - metadata, _ := json.Marshal(property) - versions := []string{string(metadata)} - vp := store.client.NewListSecretPropertiesVersionsPager(key, nil) - for vp.More() { - page, err := vp.NextPage(ctx) - if err != nil { - return Description{}, azureError(err) - } - for _, version := range page.Value { - if version == nil || version.ID == nil { - return Description{}, errors.New("incomplete Azure version metadata") - } - encoded, _ := json.Marshal(version) - versions = append(versions, string(encoded)) - } - } - if len(versions) == 1 { - return Description{}, errors.New("azure secret has no version; refusing to replace it") - } - slices.Sort(versions) - return Description{Exists: true, HasVersion: true, Version: fmt.Sprintf("%x", sha256.Sum256([]byte(strings.Join(versions, "\n"))))}, nil - } - } - return store.describeDeleted(ctx, key) -} - -func (store *AzureKeyVault) describeDeleted(ctx context.Context, key string) (Description, error) { - // Absence from the active list is insufficient: recoverable material must - // never be purged, recovered or replaced as part of credential seeding. - deleted := store.client.NewListDeletedSecretPropertiesPager(nil) - for deleted.More() { - page, err := deleted.NextPage(ctx) - if err != nil { - return Description{}, azureError(err) - } - if page.Value == nil { - return Description{}, errors.New("incomplete deleted Azure secret listing") - } - for _, property := range page.Value { - if property == nil || property.ID == nil { - return Description{}, errors.New("incomplete deleted Azure secret metadata") - } - if strings.EqualFold(property.ID.Name(), key) { - return Description{}, errors.New("azure secret is deleted and recoverable; refusing to replace it") - } - } - } - return Description{}, nil -} - -func (store *AzureKeyVault) Read(ctx context.Context, key string) (map[string]string, error) { - if !azureSecretName.MatchString(key) { - return nil, errors.New("invalid Azure Key Vault secret name") - } - result, err := store.client.GetSecret(ctx, key, "", nil) - if err != nil { - return nil, azureError(err) - } - if result.Value == nil { - return nil, errors.New("azure secret has no value") - } - return decodeDocument(key, []byte(*result.Value)) -} - -func (store *AzureKeyVault) Write(ctx context.Context, key string, document map[string]string, create bool) (string, error) { - if !azureSecretName.MatchString(key) { - return "", errors.New("invalid Azure Key Vault secret name") - } - encoded, err := json.Marshal(document) - if err != nil { - return "", errors.New("cannot encode Azure secret document") - } - value := string(encoded) - parameters := azsecrets.SetSecretParameters{Value: &value} - if !create { - current, readErr := store.client.GetSecret(ctx, key, "", nil) - if readErr != nil { - return "", azureError(readErr) - } - parameters.ContentType, parameters.Tags = current.ContentType, current.Tags - if attributes := current.Attributes; attributes != nil { - parameters.SecretAttributes = &azsecrets.SecretAttributes{ - Enabled: attributes.Enabled, Expires: attributes.Expires, NotBefore: attributes.NotBefore, - } - } - } - _, err = store.client.SetSecret(ctx, key, parameters, nil) - if err != nil { - return "", azureError(err) - } - description, err := store.Describe(ctx, key) - return description.Version, err -} - -// Never wrap SDK errors: their response bodies and auth diagnostics may contain -// credentials. This also protects %+v, %#v and JSON error formatting. -func azureError(err error) error { - if err == nil { - return nil - } - var response *azcore.ResponseError - if errors.As(err, &response) { - reason := "" - if response.StatusCode == 403 && response.RawResponse != nil { - // Inspect only the SDK's response payload for a closed diagnostic code. - // No provider text, IDs, URLs or unknown code can enter the error. - payload, readErr := runtime.Payload(response.RawResponse) - var detail struct { - Error struct { - InnerError struct { - Code string `json:"code"` - } `json:"innererror"` - } `json:"error"` - } - if readErr == nil && len(payload) <= 65536 && json.Unmarshal(payload, &detail) == nil { - switch detail.Error.InnerError.Code { - case "ForbiddenByRbac": - reason = "; data-plane role denied" - case "ForbiddenByConnection", "ForbiddenByFirewall": - reason = "; private-network access denied" - } - } - } - return fmt.Errorf("azure Key Vault request failed (HTTP %d%s; backend output withheld)", response.StatusCode, reason) - } - return errors.New("azure Key Vault authentication or transport failed (backend output withheld)") -} diff --git a/pkg/deploysecrets/azure_test.go b/pkg/deploysecrets/azure_test.go deleted file mode 100644 index c48694f33..000000000 --- a/pkg/deploysecrets/azure_test.go +++ /dev/null @@ -1,258 +0,0 @@ -package deploysecrets - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "net/url" - "strings" - "testing" - "time" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" - "github.com/codefly-dev/cli/pkg/environments" - "github.com/codefly-dev/cli/pkg/gitops" - "github.com/stretchr/testify/require" -) - -type testAzureCredential struct{} - -type azureTestTransport func(*http.Request) (*http.Response, error) - -func (f azureTestTransport) Do(r *http.Request) (*http.Response, error) { return f(r) } - -func (testAzureCredential) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) { - return azcore.AccessToken{Token: "test-only", ExpiresOn: time.Now().Add(time.Hour)}, nil -} - -func azureTestStore(t *testing.T, handler http.HandlerFunc) *AzureKeyVault { - t.Helper() - server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get("Authorization") == "" { - w.Header().Set("WWW-Authenticate", `Bearer authorization="https://login.microsoftonline.com/test", resource="https://vault.azure.net"`) - w.WriteHeader(http.StatusUnauthorized) - return - } - handler(w, r) - })) - t.Cleanup(server.Close) - endpoint, err := url.Parse(server.URL) - require.NoError(t, err) - transport := azureTestTransport(func(r *http.Request) (*http.Response, error) { - local := r.Clone(r.Context()) - local.Host, local.URL.Host = r.URL.Host, endpoint.Host - response, err := server.Client().Do(local) - if response != nil { - response.Request = r - } - return response, err - }) - client, err := azsecrets.NewClient("https://example.vault.azure.net", testAzureCredential{}, &azsecrets.ClientOptions{ - ClientOptions: azcore.ClientOptions{Transport: transport, Retry: policy.RetryOptions{MaxRetries: -1}}, - }) - require.NoError(t, err) - return &AzureKeyVault{vaultURL: server.URL, client: client} -} - -func TestAzureDescribeIsMetadataOnlyAndFollowsPages(t *testing.T) { - var paths []string - store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { - paths = append(paths, r.URL.Path) - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/secrets": - fmt.Fprintf(w, `{"value":[],"nextLink":"https://%s/page2"}`, r.Host) - case "/page2": - fmt.Fprint(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true}}]}`) - case "/secrets/KEY/versions": - fmt.Fprint(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key/v1","attributes":{"enabled":true}}]}`) - default: - t.Errorf("unexpected payload operation %s %s", r.Method, r.URL.Path) - w.WriteHeader(500) - } - }) - description, err := store.Describe(context.Background(), "KEY") - require.NoError(t, err) - require.True(t, description.Exists) - require.True(t, description.HasVersion) - require.NotEmpty(t, description.Version) - require.Equal(t, []string{"/secrets", "/page2", "/secrets/KEY/versions"}, paths) -} - -func TestAzureDescribeRefusesUnavailableMaterial(t *testing.T) { - for _, tc := range []struct { - name, active, deleted string - status int - want string - }{ - {"missing", `{"value":[]}`, `{"value":[]}`, 200, ""}, - {"disabled", `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":false}}]}`, "", 200, "disabled"}, - {"expired", `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true,"exp":1}}]}`, "", 200, "expired"}, - {"future", `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true,"nbf":4102444800}}]}`, "", 200, "not yet valid"}, - {"deleted", `{"value":[]}`, `{"value":[{"id":"https://example.vault.azure.net/secrets/key"}]}`, 200, "recoverable"}, - {"denied", `{"error":{"code":"Forbidden","message":"private-marker"}}`, "", 403, "HTTP 403"}, - {"invalid", `{"value":[{}]}`, "", 200, "incomplete"}, - {"missing-list", `{}`, "", 200, "incomplete"}, - } { - t.Run(tc.name, func(t *testing.T) { - store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(tc.status) - if r.URL.Path == "/deletedsecrets" { - fmt.Fprint(w, tc.deleted) - } else { - fmt.Fprint(w, tc.active) - } - }) - description, err := store.Describe(context.Background(), "key") - if tc.want == "" { - require.NoError(t, err) - require.False(t, description.Exists) - return - } - require.ErrorContains(t, err, tc.want) - require.NotContains(t, fmt.Sprintf("%v %+v %#v", err, err, err), "private-marker") - }) - } -} - -func TestAzurePlannerPreservesPropertiesAndRefusesChangedPlan(t *testing.T) { - for _, changed := range []bool{false, true} { - t.Run(fmt.Sprint(changed), func(t *testing.T) { - document := map[string]string{"unrelated": "keep-this"} - version, writes := 1, 0 - store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/secrets": - fmt.Fprint(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key","attributes":{"enabled":true}}]}`) - case "/secrets/key/versions": - fmt.Fprintf(w, `{"value":[{"id":"https://example.vault.azure.net/secrets/key/v%d","attributes":{"enabled":true}}]}`, version) - case "/secrets/key", "/secrets/key/": - if r.Method == http.MethodPut { - var body struct { - Value, ContentType string - Tags map[string]string - Attributes map[string]any - } - if err := json.NewDecoder(r.Body).Decode(&body); err != nil { - t.Error(err) - } - if err := json.Unmarshal([]byte(body.Value), &document); err != nil { - t.Error(err) - } - if body.Tags["owner"] != "existing-owner" || body.ContentType != "application/json" || body.Attributes["exp"] != float64(4102444800) { - t.Error("write discarded existing secret metadata") - } - writes++ - version++ - } - value, _ := json.Marshal(document) - _ = json.NewEncoder(w).Encode(map[string]any{"value": string(value), "contentType": "application/json", "tags": map[string]string{"owner": "existing-owner"}, "attributes": map[string]any{"enabled": true, "exp": 4102444800}}) - default: - t.Errorf("unexpected request %s", r.URL.Path) - w.WriteHeader(500) - } - }) - inputs := &Inputs{Rendered: gitops.RenderedEnvironment{Secrets: []gitops.RenderedServiceSecret{ - {RemoteKey: "key", Properties: filed(internalToken)}, - }}, Store: store, ReadPayloads: true, Generators: generators()} - plan, err := Build(context.Background(), inputs) - require.NoError(t, err) - if changed { - version++ - } - written, err := plan.Apply(context.Background(), store, false) - if changed { - require.ErrorContains(t, err, "changed since planning") - require.Empty(t, written) - require.Zero(t, writes) - return - } - require.NoError(t, err) - require.Len(t, written, 1) - require.Equal(t, "key", written[0].Key) - require.Equal(t, "keep-this", document["unrelated"]) - require.NotEmpty(t, document[internalToken]) - again, err := Build(context.Background(), inputs) - require.NoError(t, err) - require.Empty(t, again.Changes()) - require.Equal(t, 1, writes) - }) - } -} - -func TestResolveAzureStoreUsesESOTenantSource(t *testing.T) { - for _, tenant := range []string{"11111111-1111-1111-1111-111111111111", ""} { - runner := &scriptedRunner{answers: map[string]func() ([]byte, error){ - "kubectl get clustersecretstore cell-secrets": func() ([]byte, error) { - return []byte(`{"spec":{"provider":{"azurekv":{"vaultUrl":"https://example.vault.azure.net","authType":"WorkloadIdentity","serviceAccountRef":{"name":"eso","namespace":"external-secrets"}}}}}`), nil - }, - "kubectl get serviceaccount eso": func() ([]byte, error) { - return []byte(fmt.Sprintf(`{"metadata":{"annotations":{"azure.workload.identity/tenant-id":%q}}}`, tenant)), nil - }, - }} - store, err := ResolveStore(context.Background(), runner.run, ClusterTarget{Kubeconfig: "/kube", Context: "azure"}, - environments.EnvironmentSecretStoreReference{Name: "cell-secrets", Kind: "ClusterSecretStore"}, "app") - if tenant == "" { - require.ErrorContains(t, err, "no tenant") - continue - } - require.NoError(t, err) - require.Equal(t, "azurekv https://example.vault.azure.net", store.Name()) - require.Contains(t, runner.calls[1].args, "external-secrets") - require.Contains(t, runner.calls[1].args, "azure") - } -} - -func TestResolveAzureRefusesAmbiguousAuthority(t *testing.T) { - for _, raw := range []string{ - `{"vaultUrl":"http://example.vault.azure.net","authType":"WorkloadIdentity","tenantId":"t"}`, - `{"vaultUrl":"https://example.vault.azure.net.attacker.test","authType":"WorkloadIdentity","tenantId":"t"}`, - `{"vaultUrl":"https://example.vault.azure.net","authType":"ServicePrincipal","tenantId":"t"}`, - `{"vaultUrl":"https://example.vault.azure.net","authType":"WorkloadIdentity"}`, - `{"vaultUrl":"https://example.vault.azure.net","authType":"WorkloadIdentity","tenantId":"t","serviceAccountRef":{"name":"eso"}}`, - } { - _, err := resolveAzureStore(context.Background(), nil, ClusterTarget{}, environments.EnvironmentSecretStoreReference{Kind: "ClusterSecretStore"}, "", json.RawMessage(raw)) - require.Error(t, err) - } -} - -func TestAzureFailureNeverExposesResponseOrPayload(t *testing.T) { - store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(403) - fmt.Fprint(w, `{"error":{"code":"Forbidden","message":"private-marker"}}`) - }) - _, err := store.Write(context.Background(), "key", map[string]string{"key": "private-marker"}, true) - require.Error(t, err) - encoded, marshalErr := json.Marshal(err) - require.NoError(t, marshalErr) - for _, output := range []string{fmt.Sprintf("%v %+v %#v", err, err, err), string(encoded)} { - require.False(t, strings.Contains(output, "private-marker")) - } -} - -func TestAzureForbiddenClassifiesOnlyKnownCodes(t *testing.T) { - for code, want := range map[string]string{ - "ForbiddenByRbac": "data-plane role denied", - "ForbiddenByConnection": "private-network access denied", - "ForbiddenByFirewall": "private-network access denied", - "private-marker": "HTTP 403; backend output withheld", - } { - t.Run(code, func(t *testing.T) { - store := azureTestStore(t, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(403) - _ = json.NewEncoder(w).Encode(map[string]any{"error": map[string]any{"code": "Forbidden", "message": "private-marker", "innererror": map[string]any{"code": code}}}) - }) - _, err := store.Write(context.Background(), "key", map[string]string{"key": "private-marker"}, true) - require.ErrorContains(t, err, want) - require.NotContains(t, fmt.Sprintf("%v %+v %#v", err, err, err), "private-marker") - }) - } -} diff --git a/pkg/deploysecrets/plan.go b/pkg/deploysecrets/plan.go index 8de52666d..d794b98e8 100644 --- a/pkg/deploysecrets/plan.go +++ b/pkg/deploysecrets/plan.go @@ -11,8 +11,6 @@ // unless the operator names its generated key with --rotate (rotate.go); // 2. propagate — a configuration value is one value however many services read // it, so a key another remote secret already holds is copied from there; -// 3. supply — a key the operator handed this run, by secret key, because no -// key holds it and nothing can derive it; // 3. configured — the environment's own configuration holds the value, in the // `configurations//.secret.env` that `codefly config set` // writes (see configured.go), so the operator's existing file seeds the @@ -63,9 +61,6 @@ const ( // ActionGenerate: the property is absent and generated at random, as the // environment declares. ActionGenerate Action = "generate" - // ActionSupply: the property is absent and written from the value the - // operator supplied for its secret key. - ActionSupply Action = "supply" // ActionRequire: the property is absent and nothing can produce it; the // operator supplies it. ActionRequire Action = "require" @@ -117,7 +112,6 @@ type Plan struct { Secrets []SecretPlan writes []pendingWrite - states []*remoteState // rotating makes Apply all-or-nothing: a rotation half-written is a source // and its derived values disagreeing, so a failed write restores the rest. rotating bool @@ -203,13 +197,6 @@ type Inputs struct { // empty, from service-secrets.may-be-empty. A present-but-empty property is // otherwise refused. MayBeEmpty []string - // MintKeys explicitly acknowledges first creation of lifelong signing keys. - MintKeys []string - // Supplied are operator values for keys no remote key holds and no generator - // declares, by secret key. A supplied value never replaces a stored one and - // never stands in for a declared generator. ValidateSupplied checks them - // against the whole render first. - Supplied map[string]string // Configured are the values the environment's own configuration files hold, // by the secret key a render reads each one under — ReadConfigured. A // property no remote key holds is resolved from here before a generator is @@ -260,7 +247,6 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { if err != nil { return nil, err } - plan.states = states generators, err := generatorIndex(in.Generators, in.Services) if err != nil { @@ -291,7 +277,7 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { for _, property := range state.secret.Properties { location := propertyLocation{state.secret.RemoteKey, property.Property} propertyPlan, value, err := planConfigured(state, property, states, generators, generated, in.Configured, - aliases[location], rotating, in.MayBeEmpty, in.MintKeys, in.Supplied) + aliases[location], rotating, in.MayBeEmpty) if err != nil { return nil, err } @@ -300,7 +286,7 @@ func Build(ctx context.Context, in *Inputs) (*Plan, error) { Source: "derived: the stored URL's password follows rotated " + strings.Join(rewrite.from, ", ")} value = rewrite.value } - if value != "" || propertyPlan.Action == ActionEmpty || propertyPlan.Action == ActionSupply { + if value != "" || propertyPlan.Action == ActionEmpty { changes[property.Property] = value } secretPlan.Properties = append(secretPlan.Properties, propertyPlan) @@ -369,12 +355,8 @@ func readStates(ctx context.Context, in *Inputs) ([]*remoteState, error) { return } state := &remoteState{secret: secret, description: description} - if description.Exists && !description.HasVersion { - fail(fmt.Errorf("remote key %s exists without a readable version; refusing to regenerate retained material", secret.RemoteKey)) - return - } switch { - case !description.Exists: + case !description.Exists || !description.HasVersion: state.document = map[string]string{} case in.ReadPayloads: document, err := in.Store.Read(ctx, secret.RemoteKey) @@ -382,14 +364,6 @@ func readStates(ctx context.Context, in *Inputs) ([]*remoteState, error) { return } state.document = document - current, err := in.Store.Describe(ctx, secret.RemoteKey) - if fail(wrapKeyError("describe after read", secret.RemoteKey, err)) { - return - } - if current != description { - fail(fmt.Errorf("remote key %s changed while reading; rebuild the plan", secret.RemoteKey)) - return - } } // An empty stored value is refused, because an empty credential is // almost always a half-finished seed. Two things narrow that: @@ -561,17 +535,13 @@ func refuseSharedSigningKey(state *remoteState, property gitops.RenderedSecretPr // only consulted for a property nothing holds. func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, states []*remoteState, generators map[string]environments.EnvironmentSecretGenerator, generated map[string]string, - configured map[string]ConfiguredValue, keys []string, rotating map[string]bool, mayBeEmpty, mintKeys []string, supplied map[string]string) (PropertyPlan, string, error) { + configured map[string]ConfiguredValue, keys []string, rotating map[string]bool, mayBeEmpty []string) (PropertyPlan, string, error) { lookup := property lookup.Keys = keys declaredKey, generator, err := declaredGenerator(lookup, generators) if err != nil { return PropertyPlan{}, "", err } - suppliedValue, isSupplied, err := suppliedFor(lookup, supplied, declaredKey) - if err != nil { - return PropertyPlan{}, "", err - } configuredValue, hasConfigured, err := configuredFor(lookup, configured) if err != nil { return PropertyPlan{}, "", err @@ -581,13 +551,27 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, return PropertyPlan{}, "", err } } - holders, unread, holderValue, err := configurationHolders(state, property, lookup, states) - if err != nil { - return PropertyPlan{}, "", err - } - if len(holders) > 0 { - if err = refuseSuppliedConflict(property, isSupplied, suppliedValue, holders[0], holderValue); err != nil { - return PropertyPlan{}, "", err + var holders, unread []string + var holderValue string + for _, other := range states { + for _, candidate := range other.secret.Properties { + if (other != state || candidate.Property != property.Property) && !sharesConfigurationKey(lookup, candidate) { + continue + } + location := other.secret.RemoteKey + "#" + candidate.Property + if !other.known() { + unread = append(unread, location) + continue + } + value, present := other.document[candidate.Property] + if !present { + continue + } + if len(holders) > 0 && value != holderValue { + return PropertyPlan{}, "", fmt.Errorf("one configuration value cannot be two: different values in %s and %s; resolve it before planning", holders[0], location) + } + holders = append(holders, location) + holderValue = value } } // A configured value disagreeing with what the store holds is refused, on @@ -608,19 +592,14 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, return rotatedProperty(state, property, declaredKey, generated[declaredKey]) } if state.known() { - if _, present := state.document[property.Property]; present { + if stored, present := state.document[property.Property]; present { + if len(holders) > 0 && stored != holderValue { + return PropertyPlan{}, "", fmt.Errorf("%s holds different values in %s and %s: one configuration value cannot be two; resolve it before planning", + strings.Join(property.Keys, ", "), state.secret.RemoteKey, holders[0]) + } return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionKeep, Source: "stored"}, "", nil } } - if generator.Format == environments.SecretGeneratorFormatEd25519PKCS8 && len(holders) == 0 && len(unread) == 0 { - if state.description.Exists { - return PropertyPlan{}, "", fmt.Errorf("remote key %s exists but lacks signing key %s; restore its original material, never re-mint it", state.secret.RemoteKey, declaredKey) - } - if !slices.Contains(mintKeys, declaredKey) { - return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionRequire, - Source: "first signing-key creation requires --mint " + declaredKey}, "", nil - } - } fallback, value, err := fallbackSource(lookup, generators, generated, configuredValue, hasConfigured, mayBeEmpty) if err != nil { return PropertyPlan{}, "", err @@ -638,106 +617,11 @@ func planConfigured(state *remoteState, property gitops.RenderedSecretProperty, case len(unread) > 0: return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionUnverified, Source: fmt.Sprintf("may propagate from %s (not read); else %s", strings.Join(unread, ", "), fallback.Source)}, "", nil - case isSupplied: - return PropertyPlan{Property: property.Property, Keys: property.Keys, Action: ActionSupply, Source: "supplied by the operator"}, suppliedValue, nil default: return fallback, value, nil } } -// configurationHolders lists every location holding this property's -// configuration value — this key's own property and any other property read -// under one of its keys — and the one value they agree on. Unread keys are -// listed apart; two read holders that disagree are refused. -func configurationHolders(state *remoteState, property, lookup gitops.RenderedSecretProperty, states []*remoteState) (holders, unread []string, holderValue string, err error) { - for _, other := range states { - for _, candidate := range other.secret.Properties { - if (other != state || candidate.Property != property.Property) && !sharesConfigurationKey(lookup, candidate) { - continue - } - location := other.secret.RemoteKey + "#" + candidate.Property - if !other.known() { - unread = append(unread, location) - continue - } - value, present := other.document[candidate.Property] - if !present { - continue - } - if len(holders) > 0 && value != holderValue { - return nil, nil, "", fmt.Errorf("one configuration value cannot be two: different values in %s and %s; resolve it before planning", holders[0], location) - } - holders = append(holders, location) - holderValue = value - } - } - return holders, unread, holderValue, nil -} - -// suppliedFor is the operator's value for a property, looked up by every secret -// key it is read under. Two keys of one configuration value supplied twice must -// agree, as two stored copies must, and a declared generator is never overridden. -func suppliedFor(property gitops.RenderedSecretProperty, supplied map[string]string, declaredKey string) (string, bool, error) { - var value, from string - for _, key := range property.Keys { - candidate, ok := supplied[key] - if !ok { - continue - } - if from != "" && candidate != value { - return "", false, fmt.Errorf("one configuration value cannot be two: %s and %s are supplied differently", from, key) - } - value, from = candidate, key - } - if from != "" && declaredKey != "" { - return "", false, fmt.Errorf("%s is both supplied and declared by service-secrets.generate; drop the supplied value", declaredKey) - } - return value, from != "", nil -} - -// refuseSuppliedConflict refuses a supplied value that differs from one already -// held at location: a supplied value never replaces a stored one. -func refuseSuppliedConflict(property gitops.RenderedSecretProperty, isSupplied bool, supplied, location, held string) error { - if !isSupplied || supplied == held { - return nil - } - return fmt.Errorf("the value supplied for %s differs from the one %s holds; a supplied value never replaces a stored one", - strings.Join(property.Keys, ", "), location) -} - -// ValidateSupplied refuses supplied values the render cannot use: a key no -// rendered property reads (a typo would otherwise be dropped silently), a key -// that is not a configuration key, and an empty value for a key the environment -// does not declare may-be-empty. -func ValidateSupplied(rendered gitops.RenderedEnvironment, supplied map[string]string, mayBeEmpty []string) error { - read := map[string]bool{} - for _, secret := range rendered.Secrets { - for _, property := range secret.Properties { - for _, key := range property.Keys { - read[key] = true - } - } - } - var unknown, empty []string - for key, value := range supplied { - switch { - case !configurationKey(key) || !read[key]: - unknown = append(unknown, key) - case value == "" && !slices.Contains(mayBeEmpty, key): - empty = append(empty, key) - } - } - slices.Sort(unknown) - slices.Sort(empty) - if len(unknown) > 0 { - return fmt.Errorf("supplied keys no rendered property reads: %s", strings.Join(unknown, ", ")) - } - if len(empty) > 0 { - return fmt.Errorf("supplied empty values for keys not declared in service-secrets.may-be-empty: %s", strings.Join(empty, ", ")) - } - return nil -} - // fallbackSource is what produces a property no remote key holds: the // environment's own configuration, a declared generator, or the operator. // @@ -820,15 +704,6 @@ func (plan *Plan) Apply(ctx context.Context, store Store, allowMissing bool) ([] if err := plan.ValidateApply(allowMissing); err != nil { return nil, err } - if store.Name() != plan.Store { - return nil, fmt.Errorf("plan belongs to a different secret store") - } - // Check propagation sources as well as destinations before the first write. - for _, state := range plan.states { - if err := unchangedState(ctx, store, state); err != nil { - return nil, err - } - } var written []Written for i, write := range plan.writes { if err := unchanged(ctx, store, &write); err != nil { @@ -851,29 +726,6 @@ func (plan *Plan) Apply(ctx context.Context, store Store, allowMissing bool) ([] return written, nil } -// This is an optimistic check, not an atomic compare-and-set. Neither backend's -// version creation API offers that guarantee; operators must serialize writers. -func unchangedState(ctx context.Context, store Store, state *remoteState) error { - key := state.secret.RemoteKey - description, err := store.Describe(ctx, key) - if err != nil { - return wrapKeyError("recheck", key, err) - } - if description != state.description { - return fmt.Errorf("remote key %s changed since planning; rebuild the plan", key) - } - if description.HasVersion { - document, err := store.Read(ctx, key) - if err != nil { - return wrapKeyError("recheck", key, err) - } - if !maps.Equal(document, state.document) { - return fmt.Errorf("remote key %s changed since planning; rebuild the plan", key) - } - } - return nil -} - // unchanged refuses a write to a key that is no longer as the plan read it: a // document built from what it used to hold would silently undo whatever was // written since. The store offers no conditional version add, so this narrows diff --git a/pkg/deploysecrets/plan_test.go b/pkg/deploysecrets/plan_test.go index bf7bff201..0a8635f72 100644 --- a/pkg/deploysecrets/plan_test.go +++ b/pkg/deploysecrets/plan_test.go @@ -320,11 +320,13 @@ func TestPlanRefusesToPropagateAConflictingValue(t *testing.T) { } // With nothing stored at all, a generated shared value is minted once and is -// one value everywhere it is read. +// one value everywhere it is read; a key that exists with no enabled version is +// written like a missing one. func TestPlanOnAnEmptyStoreGeneratesOnceAndAgrees(t *testing.T) { ctx := context.Background() rendered := fixture() store := newFakeStore(map[string]map[string]string{}) + store.empty["host-accounts"] = true plan, err := Build(ctx, &Inputs{Rendered: rendered, Generators: generators(), Services: []string{"tasks/store"}, Store: store, ReadPayloads: true}) if err != nil { @@ -341,7 +343,7 @@ func TestPlanOnAnEmptyStoreGeneratesOnceAndAgrees(t *testing.T) { t.Error("a generated shared value differs between the keys reading it") } if !slices.Contains(store.writes, "host-accounts") { - t.Error("the missing key was not written") + t.Error("a key with no enabled version was not written") } } @@ -702,7 +704,7 @@ func TestAnEd25519SigningKeyIsMintedOnceForItsOneReader(t *testing.T) { Configuration: "postgres", Services: []string{"tasks/store"}, Keys: []string{"POSTGRES_PASSWORD"}, Format: environments.SecretGeneratorFormatEd25519PKCS8}} plan, err := Build(ctx, &Inputs{Rendered: rendered, Generators: keyGenerator, - Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, MintKeys: []string{storePassword}}) + Services: []string{"tasks/store"}, Store: store, ReadPayloads: true}) if err != nil { t.Fatal(err) } diff --git a/pkg/deploysecrets/retained_material_test.go b/pkg/deploysecrets/retained_material_test.go deleted file mode 100644 index 882f86309..000000000 --- a/pkg/deploysecrets/retained_material_test.go +++ /dev/null @@ -1,84 +0,0 @@ -package deploysecrets - -import ( - "context" - "errors" - "testing" - - "github.com/codefly-dev/cli/pkg/environments" - "github.com/stretchr/testify/require" -) - -type failOneWrite struct { - *fakeStore - key string -} - -func (store *failOneWrite) Write(ctx context.Context, key string, document map[string]string, create bool) (string, error) { - if key == store.key { - return "", errors.New("controlled write failure") - } - return store.fakeStore.Write(ctx, key, document, create) -} - -func TestPartialApplyReportsCompletedKeysAndRetryKeepsMaterial(t *testing.T) { - store := &failOneWrite{fakeStore: newFakeStore(seeded()), key: "tasks-store"} - inputs := &Inputs{Rendered: fixture(), Store: store, ReadPayloads: true, Generators: generators(), Services: []string{"tasks/store"}} - plan, err := Build(context.Background(), inputs) - require.NoError(t, err) - written, err := plan.Apply(context.Background(), store, true) - require.ErrorContains(t, err, "controlled write failure") - require.Len(t, written, 1) - require.Equal(t, "notes-backend", written[0].Key) - retained := store.documents["notes-backend"][internalToken] - store.key = "" - plan, err = Build(context.Background(), inputs) - require.NoError(t, err) - written, err = plan.Apply(context.Background(), store, true) - require.NoError(t, err) - require.Len(t, written, 1) - require.Equal(t, "tasks-store", written[0].Key) - require.Equal(t, retained, store.documents["notes-backend"][internalToken]) -} - -func TestPlannerRefusesExistingKeyWithoutReadableVersion(t *testing.T) { - for _, read := range []bool{false, true} { - store := newFakeStore(seeded()) - store.empty["host-accounts"] = true - _, err := Build(context.Background(), &Inputs{Rendered: fixture(), Store: store, ReadPayloads: read, Generators: generators()}) - require.ErrorContains(t, err, "without a readable version") - require.Empty(t, store.writes) - } -} - -func TestPlannerRefusesChangesBeforeApply(t *testing.T) { - for _, existing := range []bool{false, true} { - store := newFakeStore(seeded()) - if !existing { - delete(store.documents, "wiki-backend") - } - plan, err := Build(context.Background(), &Inputs{Rendered: fixture(), Store: store, ReadPayloads: true, Generators: generators()}) - require.NoError(t, err) - store.documents["wiki-backend"] = map[string]string{internalToken: "changed-after-confirmation"} - written, err := plan.Apply(context.Background(), store, true) - require.ErrorContains(t, err, "changed since planning") - require.Empty(t, written) - require.Empty(t, store.writes) - } -} - -func TestSigningKeyRequiresExplicitMintAndNeverRemintsExistingDocument(t *testing.T) { - generator := []environments.EnvironmentSecretGenerator{{Scope: environments.SecretGeneratorScopeService, - Configuration: "postgres", Services: []string{"tasks/store"}, Keys: []string{"POSTGRES_PASSWORD"}, - Format: environments.SecretGeneratorFormatEd25519PKCS8}} - store := newFakeStore(map[string]map[string]string{}) - inputs := &Inputs{Rendered: fixture(), Store: store, ReadPayloads: true, Generators: generator, Services: []string{"tasks/store"}} - plan, err := Build(context.Background(), inputs) - require.NoError(t, err) - require.Equal(t, ActionRequire, propertyPlan(t, plan, "tasks-store", storePassword).Action) - store.documents["tasks-store"] = map[string]string{"unrelated": "retained"} - inputs.MintKeys = []string{storePassword} - _, err = Build(context.Background(), inputs) - require.ErrorContains(t, err, "never re-mint") - require.Empty(t, store.writes) -} diff --git a/pkg/deploysecrets/rotate_test.go b/pkg/deploysecrets/rotate_test.go index db7bf232f..98e5dca31 100644 --- a/pkg/deploysecrets/rotate_test.go +++ b/pkg/deploysecrets/rotate_test.go @@ -329,10 +329,10 @@ func TestRotateRefusesAKeyThatMovedSinceThePlan(t *testing.T) { } store.versions = map[string]int{consumerKey: 5} _, err = plan.Apply(context.Background(), store, false) - if err == nil || !strings.Contains(err.Error(), consumerKey+" changed since planning") { + if err == nil || !strings.Contains(err.Error(), consumerKey+" changed after the plan read it") { t.Fatalf("Apply = %v, want the moved key refused by name", err) } - if store.documents[evidenceKey][evidencePassword] != oldPassword || len(store.writes) != 0 { + if store.documents[evidenceKey][evidencePassword] != oldPassword || slices.Contains(store.writes[:1], consumerKey) { t.Error("the rotation was left half-applied or overwrote the moved key") } } diff --git a/pkg/deploysecrets/store.go b/pkg/deploysecrets/store.go index 18c635820..c233aa845 100644 --- a/pkg/deploysecrets/store.go +++ b/pkg/deploysecrets/store.go @@ -38,11 +38,10 @@ type Store interface { // Description is what can be known about a remote key without reading it. type Description struct { Exists bool - // HasVersion is false when no version can be read. Existing material must - // not be treated as absent in that case. + // HasVersion is false for a key created with no enabled version: reading it + // fails, so it is planned as an empty document. HasVersion bool - // Version identifies the observed remote revision (a version ID or metadata - // fingerprint for backends without a metadata-only latest-version read). Apply compares it before + // Version identifies the newest enabled version. Apply compares it before // each write, so a key someone changed after the plan read it is refused // rather than overwritten with a document built from what it used to hold. Version string @@ -152,12 +151,6 @@ func ResolveStore(ctx context.Context, run Runner, target ClusterTarget, ref env if err := json.Unmarshal(output, &store); err != nil { return nil, fmt.Errorf("decode %s %s: invalid store document (backend output withheld)", ref.Kind, ref.Name) } - if len(store.Spec.Provider) != 1 { - return nil, fmt.Errorf("%s %s must declare exactly one secret provider", ref.Kind, ref.Name) - } - if raw, ok := store.Spec.Provider["azurekv"]; ok { - return resolveAzureStore(ctx, run, target, ref, namespace, raw) - } if raw, ok := store.Spec.Provider["gcpsm"]; ok { var gcp struct { ProjectID string `json:"projectID"` @@ -174,7 +167,7 @@ func ResolveStore(ctx context.Context, run Runner, target ClusterTarget, ref env for provider := range store.Spec.Provider { providers = append(providers, provider) } - return nil, fmt.Errorf("%s %s resolves through provider %v, which `codefly deploy secrets` cannot write yet (supported: gcpsm, azurekv)", ref.Kind, ref.Name, providers) + return nil, fmt.Errorf("%s %s resolves through provider %v, which `codefly deploy secrets` cannot write yet (supported: gcpsm)", ref.Kind, ref.Name, providers) } // GoogleSecretManager is a gcpsm-backed store, driven through the operator's own diff --git a/pkg/deploysecrets/supplied_test.go b/pkg/deploysecrets/supplied_test.go deleted file mode 100644 index 85e937616..000000000 --- a/pkg/deploysecrets/supplied_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package deploysecrets - -import ( - "context" - "strings" - "testing" -) - -const externalValue = "operator-supplied-secret" - -// The staging gap: an external client secret nothing can generate is handed in -// by the operator and written, while the rest of the plan is unchanged. -func TestASuppliedValueFillsARequiredProperty(t *testing.T) { - ctx := context.Background() - rendered := fixture() - store := newFakeStore(map[string]map[string]string{}) - supplied := map[string]string{clientSecret: externalValue} - if err := ValidateSupplied(rendered, supplied, nil); err != nil { - t.Fatal(err) - } - plan, err := Build(ctx, &Inputs{Rendered: rendered, Generators: generators(), - Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, Supplied: supplied}) - if err != nil { - t.Fatal(err) - } - got := propertyPlan(t, plan, "host-accounts", clientSecret) - if got.Action != ActionSupply || strings.Contains(got.Source, externalValue) { - t.Fatalf("plan = %+v, want supply without the value", got) - } - if _, err := plan.Apply(ctx, store, true); err != nil { - t.Fatal(err) - } - if store.documents["host-accounts"][clientSecret] != externalValue { - t.Error("the supplied value was not written") - } - // A rerun with the same value keeps it. - plan, err = Build(ctx, &Inputs{Rendered: rendered, Generators: generators(), - Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, Supplied: supplied}) - if err != nil { - t.Fatal(err) - } - if got := propertyPlan(t, plan, "host-accounts", clientSecret); got.Action != ActionKeep { - t.Errorf("rerun = %s, want keep", got.Action) - } -} - -func TestASuppliedValueNeverReplacesAStoredOne(t *testing.T) { - _, err := Build(context.Background(), &Inputs{Rendered: fixture(), Generators: generators(), - Services: []string{"tasks/store"}, Store: newFakeStore(seeded()), ReadPayloads: true, - Supplied: map[string]string{clientSecret: externalValue}}) - if err == nil || !strings.Contains(err.Error(), "never replaces") || strings.Contains(err.Error(), externalValue) { - t.Fatalf("Build = %v, want the stored value kept and the conflict refused", err) - } -} - -func TestASuppliedValueNeverStandsInForAGenerator(t *testing.T) { - _, err := Build(context.Background(), &Inputs{Rendered: fixture(), Generators: generators(), - Services: []string{"tasks/store"}, Store: newFakeStore(map[string]map[string]string{}), ReadPayloads: true, - Supplied: map[string]string{internalToken: externalValue}}) - if err == nil || !strings.Contains(err.Error(), "service-secrets.generate") { - t.Fatalf("Build = %v, want a supplied generated key refused", err) - } -} - -func TestSuppliedValuesMustBeReadAndNonEmptyUnlessDeclared(t *testing.T) { - rendered := fixture() - if err := ValidateSupplied(rendered, map[string]string{clientSecret + "_TYPO": "x"}, nil); err == nil { - t.Error("a key no property reads was accepted") - } - if err := ValidateSupplied(rendered, map[string]string{clientSecret: ""}, nil); err == nil { - t.Error("an undeclared empty value was accepted") - } - if err := ValidateSupplied(rendered, map[string]string{clientSecret: ""}, []string{clientSecret}); err != nil { - t.Errorf("a declared empty value was refused: %v", err) - } -} - -// A declared-empty key is written empty, so the ExternalSecret that reads it -// resolves instead of failing on a missing property. -func TestADeclaredEmptySuppliedValueIsWritten(t *testing.T) { - ctx := context.Background() - store := newFakeStore(map[string]map[string]string{}) - plan, err := Build(ctx, &Inputs{Rendered: fixture(), Generators: generators(), - Services: []string{"tasks/store"}, Store: store, ReadPayloads: true, - MayBeEmpty: []string{clientSecret}, Supplied: map[string]string{clientSecret: ""}}) - if err != nil { - t.Fatal(err) - } - if _, err := plan.Apply(ctx, store, true); err != nil { - t.Fatal(err) - } - if value, present := store.documents["host-accounts"][clientSecret]; !present || value != "" { - t.Errorf("declared-empty property = %q, %v; want written empty", value, present) - } -} diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index 386d94d56..3e6b09a0e 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -100,39 +100,16 @@ func RenderCell(ctx context.Context, workspace *resources.Workspace, env *enviro return result, nil } -// RenderPublishedCell inventories the committed publication trees without rebuilding -// images or rewriting their signatures. Used when binding an assembled deployment branch. -func RenderPublishedCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (CellResult, error) { - if workspace == nil || env == nil { - return CellResult{}, errors.New("a cell is rendered for a workspace and an environment") - } - root, err := cellRoot(workspace, env) - if err != nil { - return CellResult{}, err - } - file, result, err := deriveCellFrom(ctx, workspace, env, filepath.Join(root, "modules")) - if err != nil { - return CellResult{}, err - } - return result, writeCell(&file, result.Path) -} - // deriveCell is RenderCell without the write: the cell file the rendered // trees and the composition describe, and the path it belongs at. func deriveCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (cellfile.File, CellResult, error) { - return deriveCellFrom(ctx, workspace, env, "") -} - -func deriveCellFrom(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, modulesDir string) (cellfile.File, CellResult, error) { if workspace == nil || env == nil { return cellfile.File{}, CellResult{}, errors.New("a cell is rendered for a workspace and an environment") } if env.Host == nil { return cellfile.File{}, CellResult{}, fmt.Errorf("environment %s declares no host, so it has no cell to describe", env.Name) } - if modulesDir == "" { - modulesDir = filepath.Dir(moduleRenderDestination(workspace, "module")) - } + modulesDir := filepath.Dir(moduleRenderDestination(workspace, "module")) entries, err := os.ReadDir(modulesDir) if errors.Is(err, fs.ErrNotExist) { return cellfile.File{}, CellResult{}, fmt.Errorf("no module has been rendered under %s", modulesDir) diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index 25515d412..1a171ad9f 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -210,30 +210,6 @@ func renderCellTreeWith(t *testing.T, options *RenderOptions, service, environme // container port the mesh authorizes and the Service port an edge route uses, // who the composition says calls them, the ingress into them, the bindings // the environment declares, and the egress a service is granted. -func TestPublishedCellUsesPublishedTreesAndRefusesMissingModules(t *testing.T) { - workspace := writeCellWorkspace(t) - env := selectedEnvironment(t, workspace, "staging") - renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") - renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") - env.Gitops = &environments.EnvironmentGitops{RepoURL: "https://github.com/example/product", Branch: "deploy", Path: "deployments/published"} - publication := filepath.Join(workspace.Dir(), env.Gitops.Path) - require.NoError(t, os.MkdirAll(publication, 0o755)) - require.NoError(t, os.Rename(filepath.Join(workspace.Dir(), "deployments/modules"), filepath.Join(publication, "modules"))) - result, err := RenderPublishedCell(context.Background(), workspace, env) - require.NoError(t, err) - require.Equal(t, []string{"billing", "shop"}, result.Modules) - before, err := os.ReadFile(result.Path) - require.NoError(t, err) - _, err = RenderCell(context.Background(), workspace, env) - require.ErrorContains(t, err, "no module has been rendered") - require.NoError(t, os.RemoveAll(filepath.Join(publication, "modules/billing"))) - _, err = RenderPublishedCell(context.Background(), workspace, env) - require.ErrorContains(t, err, "billing") - after, err := os.ReadFile(result.Path) - require.NoError(t, err) - require.Equal(t, before, after, "incomplete publication must leave the last complete cell unchanged") -} - func TestCellFileInventoriesEveryRenderedWorkload(t *testing.T) { workspace := writeCellWorkspace(t) env := selectedEnvironment(t, workspace, "staging") diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index 1f7e58ee8..e8c2d803a 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -2201,7 +2201,6 @@ func openOrUpdatePullRequest(ctx context.Context, prepared *preparedRepository, Head: github.Ptr(prepared.plan.PromotionBranch), Base: github.Ptr(prepared.plan.BaseBranch), Body: github.Ptr(body), - Draft: github.Ptr(true), }) if err != nil { return "", 0, fmt.Errorf("open promotion pull request: %w", err) diff --git a/pkg/internal/azureauth/credential.go b/pkg/internal/azureauth/credential.go deleted file mode 100644 index 5bcddbe3e..000000000 --- a/pkg/internal/azureauth/credential.go +++ /dev/null @@ -1,50 +0,0 @@ -// Package azureauth selects one explicit Azure identity for publication. -package azureauth - -import ( - "errors" - "os" - "path/filepath" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azidentity" - "github.com/google/uuid" -) - -// Credential never falls back between an operator login and workload identity. -// The caller supplies the tenant declared by the destination, not an ambient default. -func Credential(tenant string, lookup func(string) (string, bool)) (azcore.TokenCredential, error) { - if lookup == nil { - lookup = os.LookupEnv - } - id, err := uuid.Parse(tenant) - if err != nil || id == uuid.Nil || id.String() != tenant { - return nil, errors.New("publication requires a declared Azure tenant UUID") - } - mode, _ := lookup("CODEFLY_AZURE_CREDENTIAL") - switch mode { - case "", "azure-cli": - credential, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{TenantID: tenant}) - if err != nil { - return nil, errors.New("cannot initialize the Azure CLI publication credential") - } - return credential, nil - case "workload": - workloadTenant, _ := lookup("AZURE_TENANT_ID") - client, _ := lookup("AZURE_CLIENT_ID") - tokenFile, _ := lookup("AZURE_FEDERATED_TOKEN_FILE") - clientID, err := uuid.Parse(client) - if workloadTenant != tenant || err != nil || clientID == uuid.Nil || clientID.String() != client || !filepath.IsAbs(tokenFile) { - return nil, errors.New("workload publication requires the declared Azure tenant, a client UUID and an absolute federated token file") - } - credential, err := azidentity.NewWorkloadIdentityCredential(&azidentity.WorkloadIdentityCredentialOptions{ - TenantID: tenant, ClientID: client, TokenFilePath: tokenFile, - }) - if err != nil { - return nil, errors.New("cannot initialize the Azure workload publication credential") - } - return credential, nil - default: - return nil, errors.New("CODEFLY_AZURE_CREDENTIAL must be azure-cli or workload") - } -} diff --git a/pkg/internal/azureauth/credential_test.go b/pkg/internal/azureauth/credential_test.go deleted file mode 100644 index fcae572b3..000000000 --- a/pkg/internal/azureauth/credential_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package azureauth - -import ( - "testing" - - "github.com/Azure/azure-sdk-for-go/sdk/azidentity" - "github.com/stretchr/testify/require" -) - -const tenant = "11111111-1111-1111-1111-111111111111" - -func TestCredentialUsesOnlyTheSelectedIdentity(t *testing.T) { - values := map[string]string{ - "AZURE_TENANT_ID": tenant, "AZURE_CLIENT_ID": "22222222-2222-2222-2222-222222222222", - "AZURE_FEDERATED_TOKEN_FILE": "/nonexistent/projected-token", - } - lookup := func(key string) (string, bool) { value, ok := values[key]; return value, ok } - credential, err := Credential(tenant, lookup) - require.NoError(t, err) - require.IsType(t, &azidentity.AzureCLICredential{}, credential) - values["CODEFLY_AZURE_CREDENTIAL"] = "workload" - credential, err = Credential(tenant, lookup) - require.NoError(t, err) - require.IsType(t, &azidentity.WorkloadIdentityCredential{}, credential) -} - -func TestWorkloadCredentialRefusesIncompleteOrCrossTenantIdentity(t *testing.T) { - for _, change := range []map[string]string{ - {"AZURE_TENANT_ID": "33333333-3333-3333-3333-333333333333"}, - {"AZURE_TENANT_ID": ""}, {"AZURE_CLIENT_ID": ""}, {"AZURE_CLIENT_ID": "private-marker"}, - {"AZURE_FEDERATED_TOKEN_FILE": ""}, {"AZURE_FEDERATED_TOKEN_FILE": "relative-token"}, - {"CODEFLY_AZURE_CREDENTIAL": "private-marker"}, - } { - values := map[string]string{ - "CODEFLY_AZURE_CREDENTIAL": "workload", "AZURE_TENANT_ID": tenant, - "AZURE_CLIENT_ID": "22222222-2222-2222-2222-222222222222", - "AZURE_FEDERATED_TOKEN_FILE": "/nonexistent/projected-token", - } - for key, value := range change { - values[key] = value - } - credential, err := Credential(tenant, func(key string) (string, bool) { value, ok := values[key]; return value, ok }) - require.Error(t, err) - require.Nil(t, credential) - require.NotContains(t, err.Error(), "private-marker") - } -} From 14b0a3f2da1874339f186b59779d057522cdce62 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss Date: Wed, 28 Oct 2026 21:24:18 +0000 Subject: [PATCH 6/6] fix(ci): authenticate generator agent release lookups --- .github/workflows/generate-qualification.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/generate-qualification.yml b/.github/workflows/generate-qualification.yml index 5feae773a..9fb0c07e7 100644 --- a/.github/workflows/generate-qualification.yml +++ b/.github/workflows/generate-qualification.yml @@ -101,6 +101,11 @@ jobs: "$RUNNER_TEMP/generate-bin/buf" --version echo "$RUNNER_TEMP/generate-bin" >> "$GITHUB_PATH" - name: Run tagged and qualification tests in isolated processes + env: + # Core uses this read-only token for release lookups and downloads. + # Each test has its own CODEFLY_HOME, so unauthenticated lookups + # otherwise exhaust the runner's shared GitHub API quota. + GITHUB_TOKEN: ${{ github.token }} run: ./scripts/test-generate-isolated.sh "${{ matrix.shard }}" "${{ strategy.job-total }}" - name: Retain test output, timings and conformance receipts if: always()