fix: harden pass migration auto-fixes
- commit
- aba13a41f99c4d1d5bdcb1227b498ef3adf1a94f
- author
- brice · · 2026-04-26 18:18 UTC
- parent
- c573276
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +152 -44
M internal/adapters/out/domainsecrets/pass_store.go +111 -0
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "regexp" | |
| 9 | 9 | "sort" | |
| 10 | 10 | "strings" | |
| 11 | + | "sync" | |
| 11 | 12 | "time" | |
| 12 | 13 | ||
| 13 | 14 | "github.com/bnema/zerowrap" | |
| @@ -27,6 +28,7 @@ const ( | |||
| 27 | 28 | ||
| 28 | 29 | // PassStore implements the DomainSecretStore interface using the pass password manager. | |
| 29 | 30 | type PassStore struct { | |
| 31 | + | mu sync.Mutex | |
| 30 | 32 | timeout time.Duration | |
| 31 | 33 | log zerowrap.Logger | |
| 32 | 34 | } | |
| @@ -138,6 +140,26 @@ func (s *PassStore) GetAll(domainName string) (map[string]string, error) { | |||
| 138 | 140 | return secretsMap, nil | |
| 139 | 141 | } | |
| 140 | 142 | ||
| 143 | + | // SetIfEmpty sets multiple secrets for a domain only when no keys already exist. | |
| 144 | + | // It performs the precondition check inside the store adapter so migration | |
| 145 | + | // callers cannot accidentally split the check and write into separate steps. | |
| 146 | + | func (s *PassStore) SetIfEmpty(domainName string, secretsMap map[string]string) ([]string, error) { | |
| 147 | + | s.mu.Lock() | |
| 148 | + | defer s.mu.Unlock() | |
| 149 | + | ||
| 150 | + | existingKeys, err := s.ListKeys(domainName) | |
| 151 | + | if err != nil { | |
| 152 | + | return nil, err | |
| 153 | + | } | |
| 154 | + | if len(existingKeys) > 0 { | |
| 155 | + | return nil, fmt.Errorf("%w: pass secrets already exist for %s (found %d keys)", domain.ErrSecretsAlreadyExist, domainName, len(existingKeys)) | |
| 156 | + | } | |
| 157 | + | if err := s.Set(domainName, secretsMap); err != nil { | |
| 158 | + | return nil, err | |
| 159 | + | } | |
| 160 | + | return sortedMapKeys(secretsMap), nil | |
| 161 | + | } | |
| 162 | + | ||
| 141 | 163 | // Set sets or updates multiple secrets for a domain. | |
| 142 | 164 | func (s *PassStore) Set(domainName string, secretsMap map[string]string) error { | |
| 143 | 165 | if _, err := s.domainPath(domainName); err != nil { | |
| @@ -253,6 +275,26 @@ func (s *PassStore) Delete(domainName, key string) error { | |||
| 253 | 275 | return nil | |
| 254 | 276 | } | |
| 255 | 277 | ||
| 278 | + | // SetAttachmentIfEmpty sets attachment secrets only when no keys already exist. | |
| 279 | + | // It performs the precondition check inside the store adapter so migration | |
| 280 | + | // callers cannot accidentally split the check and write into separate steps. | |
| 281 | + | func (s *PassStore) SetAttachmentIfEmpty(containerName string, secretsMap map[string]string) ([]string, error) { | |
| 282 | + | s.mu.Lock() | |
| 283 | + | defer s.mu.Unlock() | |
| 284 | + | ||
| 285 | + | existingKeys, err := s.listAttachmentKeysRecover(containerName) | |
| 286 | + | if err != nil { | |
| 287 | + | return nil, err | |
| 288 | + | } | |
| 289 | + | if len(existingKeys) > 0 { | |
| 290 | + | return nil, fmt.Errorf("%w: pass secrets already exist for attachment %s (found %d keys)", domain.ErrSecretsAlreadyExist, containerName, len(existingKeys)) | |
| 291 | + | } | |
| 292 | + | if err := s.SetAttachment(containerName, secretsMap); err != nil { | |
| 293 | + | return nil, err | |
| 294 | + | } | |
| 295 | + | return sortedMapKeys(secretsMap), nil | |
| 296 | + | } | |
| 297 | + | ||
| 256 | 298 | // SetAttachment sets or updates multiple secrets for an attachment container. | |
| 257 | 299 | func (s *PassStore) SetAttachment(containerName string, secretsMap map[string]string) error { | |
| 258 | 300 | if _, err := s.attachmentPath(containerName); err != nil { | |
| @@ -613,6 +655,66 @@ func (s *PassStore) listAttachmentKeys(containerName string) ([]string, error) { | |||
| 613 | 655 | keys = append(keys, key) | |
| 614 | 656 | } | |
| 615 | 657 | ||
| 658 | + | sort.Strings(keys) | |
| 659 | + | return keys, nil | |
| 660 | + | } | |
| 661 | + | ||
| 662 | + | func (s *PassStore) listAttachmentKeysRecover(containerName string) ([]string, error) { | |
| 663 | + | keys, err := s.listAttachmentKeys(containerName) | |
| 664 | + | if err != nil { | |
| 665 | + | return nil, err | |
| 666 | + | } | |
| 667 | + | discovered, err := s.discoverAttachmentKeys(containerName) | |
| 668 | + | if err != nil { | |
| 669 | + | return nil, err | |
| 670 | + | } | |
| 671 | + | merged, changed := mergeUniqueKeys(keys, discovered) | |
| 672 | + | if changed { | |
| 673 | + | manifestPath, err := s.attachmentManifestPath(containerName) | |
| 674 | + | if err != nil { | |
| 675 | + | return nil, err | |
| 676 | + | } | |
| 677 | + | writeCtx, writeCancel := context.WithTimeout(context.Background(), s.timeout) | |
| 678 | + | writeErr := s.passInsert(writeCtx, manifestPath, strings.Join(merged, "\n")) | |
| 679 | + | writeCancel() | |
| 680 | + | if writeErr != nil { | |
| 681 | + | s.log.Warn(). | |
| 682 | + | Str(zerowrap.FieldLayer, "adapter"). | |
| 683 | + | Str(zerowrap.FieldAdapter, "domainsecrets"). | |
| 684 | + | Str("container", containerName). | |
| 685 | + | Err(writeErr). | |
| 686 | + | Msg("failed to self-heal attachment pass manifest, continuing with recovered keys") | |
| 687 | + | } | |
| 688 | + | } | |
| 689 | + | return merged, nil | |
| 690 | + | } | |
| 691 | + | ||
| 692 | + | func (s *PassStore) discoverAttachmentKeys(containerName string) ([]string, error) { | |
| 693 | + | basePath, err := s.attachmentPath(containerName) | |
| 694 | + | if err != nil { | |
| 695 | + | return nil, err | |
| 696 | + | } | |
| 697 | + | ||
| 698 | + | ctx, cancel := context.WithTimeout(context.Background(), s.timeout) | |
| 699 | + | defer cancel() | |
| 700 | + | ||
| 701 | + | entries, err := s.listTopLevelEntries(ctx, basePath) | |
| 702 | + | if err != nil { | |
| 703 | + | return nil, err | |
| 704 | + | } | |
| 705 | + | ||
| 706 | + | keys := make([]string, 0, len(entries)) | |
| 707 | + | for _, entry := range entries { | |
| 708 | + | if entry == ".keys" { | |
| 709 | + | continue | |
| 710 | + | } | |
| 711 | + | if err := domain.ValidateEnvKey(entry); err != nil { | |
| 712 | + | continue | |
| 713 | + | } | |
| 714 | + | keys = append(keys, entry) | |
| 715 | + | } | |
| 716 | + | ||
| 717 | + | sort.Strings(keys) | |
| 616 | 718 | return keys, nil | |
| 617 | 719 | } | |
| 618 | 720 | ||
| @@ -778,6 +880,15 @@ func passEntryMissing(output string) bool { | |||
| 778 | 880 | strings.Contains(lower, "password store is empty") | |
| 779 | 881 | } | |
| 780 | 882 | ||
| 883 | + | func sortedMapKeys(values map[string]string) []string { | |
| 884 | + | keys := make([]string, 0, len(values)) | |
| 885 | + | for key := range values { | |
| 886 | + | keys = append(keys, key) | |
| 887 | + | } | |
| 888 | + | sort.Strings(keys) | |
| 889 | + | return keys | |
| 890 | + | } | |
| 891 | + | ||
| 781 | 892 | func mergeUniqueKeys(primary, secondary []string) ([]string, bool) { | |
| 782 | 893 | mergedSet := make(map[string]struct{}, len(primary)+len(secondary)) | |
| 783 | 894 | for _, key := range primary { | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "regexp" | 8 | "regexp" | ||
| 9 | "sort" | 9 | "sort" | ||
| 10 | "strings" | 10 | "strings" | ||
| 11 | + | "sync" | |||
| 11 | "time" | 12 | "time" | ||
| 12 | 13 | ||||
| 13 | "github.com/bnema/zerowrap" | 14 | "github.com/bnema/zerowrap" | ||
| @@ -27,6 +28,7 @@ const ( | |||||
| 27 | 28 | ||||
| 28 | // PassStore implements the DomainSecretStore interface using the pass password manager. | 29 | // PassStore implements the DomainSecretStore interface using the pass password manager. | ||
| 29 | type PassStore struct { | 30 | type PassStore struct { | ||
| 31 | + | mu sync.Mutex | |||
| 30 | timeout time.Duration | 32 | timeout time.Duration | ||
| 31 | log zerowrap.Logger | 33 | log zerowrap.Logger | ||
| 32 | } | 34 | } | ||
| @@ -138,6 +140,26 @@ func (s *PassStore) GetAll(domainName string) (map[string]string, error) { | |||||
| 138 | return secretsMap, nil | 140 | return secretsMap, nil | ||
| 139 | } | 141 | } | ||
| 140 | 142 | ||||
| 143 | + | // SetIfEmpty sets multiple secrets for a domain only when no keys already exist. | |||
| 144 | + | // It performs the precondition check inside the store adapter so migration | |||
| 145 | + | // callers cannot accidentally split the check and write into separate steps. | |||
| 146 | + | func (s *PassStore) SetIfEmpty(domainName string, secretsMap map[string]string) ([]string, error) { | |||
| 147 | + | s.mu.Lock() | |||
| 148 | + | defer s.mu.Unlock() | |||
| 149 | + | ||||
| 150 | + | existingKeys, err := s.ListKeys(domainName) | |||
| 151 | + | if err != nil { | |||
| 152 | + | return nil, err | |||
| 153 | + | } | |||
| 154 | + | if len(existingKeys) > 0 { | |||
| 155 | + | return nil, fmt.Errorf("%w: pass secrets already exist for %s (found %d keys)", domain.ErrSecretsAlreadyExist, domainName, len(existingKeys)) | |||
| 156 | + | } | |||
| 157 | + | if err := s.Set(domainName, secretsMap); err != nil { | |||
| 158 | + | return nil, err | |||
| 159 | + | } | |||
| 160 | + | return sortedMapKeys(secretsMap), nil | |||
| 161 | + | } | |||
| 162 | + | ||||
| 141 | // Set sets or updates multiple secrets for a domain. | 163 | // Set sets or updates multiple secrets for a domain. | ||
| 142 | func (s *PassStore) Set(domainName string, secretsMap map[string]string) error { | 164 | func (s *PassStore) Set(domainName string, secretsMap map[string]string) error { | ||
| 143 | if _, err := s.domainPath(domainName); err != nil { | 165 | if _, err := s.domainPath(domainName); err != nil { | ||
| @@ -253,6 +275,26 @@ func (s *PassStore) Delete(domainName, key string) error { | |||||
| 253 | return nil | 275 | return nil | ||
| 254 | } | 276 | } | ||
| 255 | 277 | ||||
| 278 | + | // SetAttachmentIfEmpty sets attachment secrets only when no keys already exist. | |||
| 279 | + | // It performs the precondition check inside the store adapter so migration | |||
| 280 | + | // callers cannot accidentally split the check and write into separate steps. | |||
| 281 | + | func (s *PassStore) SetAttachmentIfEmpty(containerName string, secretsMap map[string]string) ([]string, error) { | |||
| 282 | + | s.mu.Lock() | |||
| 283 | + | defer s.mu.Unlock() | |||
| 284 | + | ||||
| 285 | + | existingKeys, err := s.listAttachmentKeysRecover(containerName) | |||
| 286 | + | if err != nil { | |||
| 287 | + | return nil, err | |||
| 288 | + | } | |||
| 289 | + | if len(existingKeys) > 0 { | |||
| 290 | + | return nil, fmt.Errorf("%w: pass secrets already exist for attachment %s (found %d keys)", domain.ErrSecretsAlreadyExist, containerName, len(existingKeys)) | |||
| 291 | + | } | |||
| 292 | + | if err := s.SetAttachment(containerName, secretsMap); err != nil { | |||
| 293 | + | return nil, err | |||
| 294 | + | } | |||
| 295 | + | return sortedMapKeys(secretsMap), nil | |||
| 296 | + | } | |||
| 297 | + | ||||
| 256 | // SetAttachment sets or updates multiple secrets for an attachment container. | 298 | // SetAttachment sets or updates multiple secrets for an attachment container. | ||
| 257 | func (s *PassStore) SetAttachment(containerName string, secretsMap map[string]string) error { | 299 | func (s *PassStore) SetAttachment(containerName string, secretsMap map[string]string) error { | ||
| 258 | if _, err := s.attachmentPath(containerName); err != nil { | 300 | if _, err := s.attachmentPath(containerName); err != nil { | ||
| @@ -613,6 +655,66 @@ func (s *PassStore) listAttachmentKeys(containerName string) ([]string, error) { | |||||
| 613 | keys = append(keys, key) | 655 | keys = append(keys, key) | ||
| 614 | } | 656 | } | ||
| 615 | 657 | ||||
| 658 | + | sort.Strings(keys) | |||
| 659 | + | return keys, nil | |||
| 660 | + | } | |||
| 661 | + | ||||
| 662 | + | func (s *PassStore) listAttachmentKeysRecover(containerName string) ([]string, error) { | |||
| 663 | + | keys, err := s.listAttachmentKeys(containerName) | |||
| 664 | + | if err != nil { | |||
| 665 | + | return nil, err | |||
| 666 | + | } | |||
| 667 | + | discovered, err := s.discoverAttachmentKeys(containerName) | |||
| 668 | + | if err != nil { | |||
| 669 | + | return nil, err | |||
| 670 | + | } | |||
| 671 | + | merged, changed := mergeUniqueKeys(keys, discovered) | |||
| 672 | + | if changed { | |||
| 673 | + | manifestPath, err := s.attachmentManifestPath(containerName) | |||
| 674 | + | if err != nil { | |||
| 675 | + | return nil, err | |||
| 676 | + | } | |||
| 677 | + | writeCtx, writeCancel := context.WithTimeout(context.Background(), s.timeout) | |||
| 678 | + | writeErr := s.passInsert(writeCtx, manifestPath, strings.Join(merged, "\n")) | |||
| 679 | + | writeCancel() | |||
| 680 | + | if writeErr != nil { | |||
| 681 | + | s.log.Warn(). | |||
| 682 | + | Str(zerowrap.FieldLayer, "adapter"). | |||
| 683 | + | Str(zerowrap.FieldAdapter, "domainsecrets"). | |||
| 684 | + | Str("container", containerName). | |||
| 685 | + | Err(writeErr). | |||
| 686 | + | Msg("failed to self-heal attachment pass manifest, continuing with recovered keys") | |||
| 687 | + | } | |||
| 688 | + | } | |||
| 689 | + | return merged, nil | |||
| 690 | + | } | |||
| 691 | + | ||||
| 692 | + | func (s *PassStore) discoverAttachmentKeys(containerName string) ([]string, error) { | |||
| 693 | + | basePath, err := s.attachmentPath(containerName) | |||
| 694 | + | if err != nil { | |||
| 695 | + | return nil, err | |||
| 696 | + | } | |||
| 697 | + | ||||
| 698 | + | ctx, cancel := context.WithTimeout(context.Background(), s.timeout) | |||
| 699 | + | defer cancel() | |||
| 700 | + | ||||
| 701 | + | entries, err := s.listTopLevelEntries(ctx, basePath) | |||
| 702 | + | if err != nil { | |||
| 703 | + | return nil, err | |||
| 704 | + | } | |||
| 705 | + | ||||
| 706 | + | keys := make([]string, 0, len(entries)) | |||
| 707 | + | for _, entry := range entries { | |||
| 708 | + | if entry == ".keys" { | |||
| 709 | + | continue | |||
| 710 | + | } | |||
| 711 | + | if err := domain.ValidateEnvKey(entry); err != nil { | |||
| 712 | + | continue | |||
| 713 | + | } | |||
| 714 | + | keys = append(keys, entry) | |||
| 715 | + | } | |||
| 716 | + | ||||
| 717 | + | sort.Strings(keys) | |||
| 616 | return keys, nil | 718 | return keys, nil | ||
| 617 | } | 719 | } | ||
| 618 | 720 | ||||
| @@ -778,6 +880,15 @@ func passEntryMissing(output string) bool { | |||||
| 778 | strings.Contains(lower, "password store is empty") | 880 | strings.Contains(lower, "password store is empty") | ||
| 779 | } | 881 | } | ||
| 780 | 882 | ||||
| 883 | + | func sortedMapKeys(values map[string]string) []string { | |||
| 884 | + | keys := make([]string, 0, len(values)) | |||
| 885 | + | for key := range values { | |||
| 886 | + | keys = append(keys, key) | |||
| 887 | + | } | |||
| 888 | + | sort.Strings(keys) | |||
| 889 | + | return keys | |||
| 890 | + | } | |||
| 891 | + | ||||
| 781 | func mergeUniqueKeys(primary, secondary []string) ([]string, bool) { | 892 | func mergeUniqueKeys(primary, secondary []string) ([]string, bool) { | ||
| 782 | mergedSet := make(map[string]struct{}, len(primary)+len(secondary)) | 893 | mergedSet := make(map[string]struct{}, len(primary)+len(secondary)) | ||
| 783 | for _, key := range primary { | 894 | for _, key := range primary { | ||
M internal/app/migrate_env.go +27 -44
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package app | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "errors" | |
| 4 | 5 | "fmt" | |
| 5 | 6 | "os" | |
| 6 | 7 | "path/filepath" | |
| @@ -79,15 +80,11 @@ func migrateEnvFile(envDir, name string, passStore *domainsecrets.PassStore, log | |||
| 79 | 80 | return nil | |
| 80 | 81 | } | |
| 81 | 82 | ||
| 82 | - | existingKeys, err := passStore.ListKeys(domainName) | |
| 83 | + | writtenKeys, err := passStore.SetIfEmpty(domainName, secrets) | |
| 83 | 84 | if err != nil { | |
| 84 | - | return fmt.Errorf("failed to read pass keys for %s: %w", domainName, err) | |
| 85 | - | } | |
| 86 | - | ||
| 87 | - | if len(existingKeys) > 0 { | |
| 88 | - | return fmt.Errorf("%w: pass secrets already exist for %s (found %d keys); refusing to delete plaintext env file automatically", domain.ErrSecretsAlreadyExist, domainName, len(existingKeys)) | |
| 89 | - | } | |
| 90 | - | if err := passStore.Set(domainName, secrets); err != nil { | |
| 85 | + | if errors.Is(err, domain.ErrSecretsAlreadyExist) { | |
| 86 | + | return fmt.Errorf("%w; refusing to delete plaintext env file automatically", err) | |
| 87 | + | } | |
| 91 | 88 | return fmt.Errorf("failed to migrate secrets for %s: %w", domainName, err) | |
| 92 | 89 | } | |
| 93 | 90 | ||
| @@ -102,17 +99,14 @@ func migrateEnvFile(envDir, name string, passStore *domainsecrets.PassStore, log | |||
| 102 | 99 | Msg("secrets migrated successfully but failed to remove original file; treating as success") | |
| 103 | 100 | return nil | |
| 104 | 101 | } | |
| 105 | - | // Attempt rollback: remove the pass entry we just wrote | |
| 106 | - | rollbackKeys, listErr := passStore.ListKeys(domainName) | |
| 107 | - | if listErr == nil { | |
| 108 | - | for _, key := range rollbackKeys { | |
| 109 | - | if delErr := passStore.Delete(domainName, key); delErr != nil { | |
| 110 | - | log.Error(). | |
| 111 | - | Err(delErr). | |
| 112 | - | Str("domain", domainName). | |
| 113 | - | Str("key", key). | |
| 114 | - | Msg("failed to rollback pass entry after file removal failure") | |
| 115 | - | } | |
| 102 | + | // Attempt rollback: remove only the pass entries this migration wrote. | |
| 103 | + | for _, key := range writtenKeys { | |
| 104 | + | if delErr := passStore.Delete(domainName, key); delErr != nil { | |
| 105 | + | log.Error(). | |
| 106 | + | Err(delErr). | |
| 107 | + | Str("domain", domainName). | |
| 108 | + | Str("key", key). | |
| 109 | + | Msg("failed to rollback pass entry after file removal failure") | |
| 116 | 110 | } | |
| 117 | 111 | } | |
| 118 | 112 | return fmt.Errorf("failed to remove migrated env file %s: %w", filePath, err) | |
| @@ -174,20 +168,11 @@ func migrateAttachmentEnvFile(envDir, name string, passStore *domainsecrets.Pass | |||
| 174 | 168 | return nil | |
| 175 | 169 | } | |
| 176 | 170 | ||
| 177 | - | existingSecrets, err := passStore.GetAllAttachment(containerName) | |
| 171 | + | writtenKeys, err := passStore.SetAttachmentIfEmpty(containerName, secrets) | |
| 178 | 172 | if err != nil { | |
| 179 | - | return fmt.Errorf("failed to read pass secrets for attachment %s: %w", containerName, err) | |
| 180 | - | } | |
| 181 | - | ||
| 182 | - | existingKeys := make([]string, 0, len(existingSecrets)) | |
| 183 | - | for key := range existingSecrets { | |
| 184 | - | existingKeys = append(existingKeys, key) | |
| 185 | - | } | |
| 186 | - | ||
| 187 | - | if len(existingKeys) > 0 { | |
| 188 | - | return fmt.Errorf("%w: pass secrets already exist for attachment %s (found %d keys); refusing to delete plaintext env file automatically", domain.ErrSecretsAlreadyExist, containerName, len(existingKeys)) | |
| 189 | - | } | |
| 190 | - | if err := passStore.SetAttachment(containerName, secrets); err != nil { | |
| 173 | + | if errors.Is(err, domain.ErrSecretsAlreadyExist) { | |
| 174 | + | return fmt.Errorf("%w; refusing to delete plaintext env file automatically", err) | |
| 175 | + | } | |
| 191 | 176 | return fmt.Errorf("failed to migrate attachment secrets for %s: %w", containerName, err) | |
| 192 | 177 | } | |
| 193 | 178 | ||
| @@ -202,17 +187,14 @@ func migrateAttachmentEnvFile(envDir, name string, passStore *domainsecrets.Pass | |||
| 202 | 187 | Msg("attachment secrets migrated successfully but failed to remove original file; treating as success") | |
| 203 | 188 | return nil | |
| 204 | 189 | } | |
| 205 | - | // Attempt rollback: remove the pass entry we just wrote | |
| 206 | - | rollbackSecrets, listErr := passStore.GetAllAttachment(containerName) | |
| 207 | - | if listErr == nil { | |
| 208 | - | for key := range rollbackSecrets { | |
| 209 | - | if delErr := passStore.DeleteAttachment(containerName, key); delErr != nil { | |
| 210 | - | log.Error(). | |
| 211 | - | Err(delErr). | |
| 212 | - | Str("container", containerName). | |
| 213 | - | Str("key", key). | |
| 214 | - | Msg("failed to rollback pass entry after file removal failure") | |
| 215 | - | } | |
| 190 | + | // Attempt rollback: remove only the pass entries this migration wrote. | |
| 191 | + | for _, key := range writtenKeys { | |
| 192 | + | if delErr := passStore.DeleteAttachment(containerName, key); delErr != nil { | |
| 193 | + | log.Error(). | |
| 194 | + | Err(delErr). | |
| 195 | + | Str("container", containerName). | |
| 196 | + | Str("key", key). | |
| 197 | + | Msg("failed to rollback pass entry after file removal failure") | |
| 216 | 198 | } | |
| 217 | 199 | } | |
| 218 | 200 | return fmt.Errorf("failed to remove migrated attachment env file %s: %w", filePath, err) | |
| @@ -237,7 +219,8 @@ func secretsMatch(a, b map[string]string) bool { | |||
| 237 | 219 | return false | |
| 238 | 220 | } | |
| 239 | 221 | for k, v := range a { | |
| 240 | - | if b[k] != v { | |
| 222 | + | bv, ok := b[k] | |
| 223 | + | if !ok || bv != v { | |
| 241 | 224 | return false | |
| 242 | 225 | } | |
| 243 | 226 | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package app | 1 | package app | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "errors" | |||
| 4 | "fmt" | 5 | "fmt" | ||
| 5 | "os" | 6 | "os" | ||
| 6 | "path/filepath" | 7 | "path/filepath" | ||
| @@ -79,15 +80,11 @@ func migrateEnvFile(envDir, name string, passStore *domainsecrets.PassStore, log | |||||
| 79 | return nil | 80 | return nil | ||
| 80 | } | 81 | } | ||
| 81 | 82 | ||||
| 82 | - | existingKeys, err := passStore.ListKeys(domainName) | 83 | + | writtenKeys, err := passStore.SetIfEmpty(domainName, secrets) |
| 83 | if err != nil { | 84 | if err != nil { | ||
| 84 | - | return fmt.Errorf("failed to read pass keys for %s: %w", domainName, err) | 85 | + | if errors.Is(err, domain.ErrSecretsAlreadyExist) { |
| 85 | - | } | 86 | + | return fmt.Errorf("%w; refusing to delete plaintext env file automatically", err) |
| 86 | - | 87 | + | } | |
| 87 | - | if len(existingKeys) > 0 { | |||
| 88 | - | return fmt.Errorf("%w: pass secrets already exist for %s (found %d keys); refusing to delete plaintext env file automatically", domain.ErrSecretsAlreadyExist, domainName, len(existingKeys)) | |||
| 89 | - | } | |||
| 90 | - | if err := passStore.Set(domainName, secrets); err != nil { | |||
| 91 | return fmt.Errorf("failed to migrate secrets for %s: %w", domainName, err) | 88 | return fmt.Errorf("failed to migrate secrets for %s: %w", domainName, err) | ||
| 92 | } | 89 | } | ||
| 93 | 90 | ||||
| @@ -102,17 +99,14 @@ func migrateEnvFile(envDir, name string, passStore *domainsecrets.PassStore, log | |||||
| 102 | Msg("secrets migrated successfully but failed to remove original file; treating as success") | 99 | Msg("secrets migrated successfully but failed to remove original file; treating as success") | ||
| 103 | return nil | 100 | return nil | ||
| 104 | } | 101 | } | ||
| 105 | - | // Attempt rollback: remove the pass entry we just wrote | 102 | + | // Attempt rollback: remove only the pass entries this migration wrote. |
| 106 | - | rollbackKeys, listErr := passStore.ListKeys(domainName) | 103 | + | for _, key := range writtenKeys { |
| 107 | - | if listErr == nil { | 104 | + | if delErr := passStore.Delete(domainName, key); delErr != nil { |
| 108 | - | for _, key := range rollbackKeys { | 105 | + | log.Error(). |
| 109 | - | if delErr := passStore.Delete(domainName, key); delErr != nil { | 106 | + | Err(delErr). |
| 110 | - | log.Error(). | 107 | + | Str("domain", domainName). |
| 111 | - | Err(delErr). | 108 | + | Str("key", key). |
| 112 | - | Str("domain", domainName). | 109 | + | Msg("failed to rollback pass entry after file removal failure") |
| 113 | - | Str("key", key). | |||
| 114 | - | Msg("failed to rollback pass entry after file removal failure") | |||
| 115 | - | } | |||
| 116 | } | 110 | } | ||
| 117 | } | 111 | } | ||
| 118 | return fmt.Errorf("failed to remove migrated env file %s: %w", filePath, err) | 112 | return fmt.Errorf("failed to remove migrated env file %s: %w", filePath, err) | ||
| @@ -174,20 +168,11 @@ func migrateAttachmentEnvFile(envDir, name string, passStore *domainsecrets.Pass | |||||
| 174 | return nil | 168 | return nil | ||
| 175 | } | 169 | } | ||
| 176 | 170 | ||||
| 177 | - | existingSecrets, err := passStore.GetAllAttachment(containerName) | 171 | + | writtenKeys, err := passStore.SetAttachmentIfEmpty(containerName, secrets) |
| 178 | if err != nil { | 172 | if err != nil { | ||
| 179 | - | return fmt.Errorf("failed to read pass secrets for attachment %s: %w", containerName, err) | 173 | + | if errors.Is(err, domain.ErrSecretsAlreadyExist) { |
| 180 | - | } | 174 | + | return fmt.Errorf("%w; refusing to delete plaintext env file automatically", err) |
| 181 | - | 175 | + | } | |
| 182 | - | existingKeys := make([]string, 0, len(existingSecrets)) | |||
| 183 | - | for key := range existingSecrets { | |||
| 184 | - | existingKeys = append(existingKeys, key) | |||
| 185 | - | } | |||
| 186 | - | ||||
| 187 | - | if len(existingKeys) > 0 { | |||
| 188 | - | return fmt.Errorf("%w: pass secrets already exist for attachment %s (found %d keys); refusing to delete plaintext env file automatically", domain.ErrSecretsAlreadyExist, containerName, len(existingKeys)) | |||
| 189 | - | } | |||
| 190 | - | if err := passStore.SetAttachment(containerName, secrets); err != nil { | |||
| 191 | return fmt.Errorf("failed to migrate attachment secrets for %s: %w", containerName, err) | 176 | return fmt.Errorf("failed to migrate attachment secrets for %s: %w", containerName, err) | ||
| 192 | } | 177 | } | ||
| 193 | 178 | ||||
| @@ -202,17 +187,14 @@ func migrateAttachmentEnvFile(envDir, name string, passStore *domainsecrets.Pass | |||||
| 202 | Msg("attachment secrets migrated successfully but failed to remove original file; treating as success") | 187 | Msg("attachment secrets migrated successfully but failed to remove original file; treating as success") | ||
| 203 | return nil | 188 | return nil | ||
| 204 | } | 189 | } | ||
| 205 | - | // Attempt rollback: remove the pass entry we just wrote | 190 | + | // Attempt rollback: remove only the pass entries this migration wrote. |
| 206 | - | rollbackSecrets, listErr := passStore.GetAllAttachment(containerName) | 191 | + | for _, key := range writtenKeys { |
| 207 | - | if listErr == nil { | 192 | + | if delErr := passStore.DeleteAttachment(containerName, key); delErr != nil { |
| 208 | - | for key := range rollbackSecrets { | 193 | + | log.Error(). |
| 209 | - | if delErr := passStore.DeleteAttachment(containerName, key); delErr != nil { | 194 | + | Err(delErr). |
| 210 | - | log.Error(). | 195 | + | Str("container", containerName). |
| 211 | - | Err(delErr). | 196 | + | Str("key", key). |
| 212 | - | Str("container", containerName). | 197 | + | Msg("failed to rollback pass entry after file removal failure") |
| 213 | - | Str("key", key). | |||
| 214 | - | Msg("failed to rollback pass entry after file removal failure") | |||
| 215 | - | } | |||
| 216 | } | 198 | } | ||
| 217 | } | 199 | } | ||
| 218 | return fmt.Errorf("failed to remove migrated attachment env file %s: %w", filePath, err) | 200 | return fmt.Errorf("failed to remove migrated attachment env file %s: %w", filePath, err) | ||
| @@ -237,7 +219,8 @@ func secretsMatch(a, b map[string]string) bool { | |||||
| 237 | return false | 219 | return false | ||
| 238 | } | 220 | } | ||
| 239 | for k, v := range a { | 221 | for k, v := range a { | ||
| 240 | - | if b[k] != v { | 222 | + | bv, ok := b[k] |
| 223 | + | if !ok || bv != v { | |||
| 241 | return false | 224 | return false | ||
| 242 | } | 225 | } | ||
| 243 | } | 226 | } | ||
M internal/usecase/container/service.go +4 -0
| @@ -2784,6 +2784,10 @@ func (s *Service) resolveExistingAttachment(ctx context.Context, ownerDomain, se | |||
| 2784 | 2784 | // Utility functions | |
| 2785 | 2785 | ||
| 2786 | 2786 | func normalizeImageRef(image string) string { | |
| 2787 | + | if isDigestRef(image) { | |
| 2788 | + | return image | |
| 2789 | + | } | |
| 2790 | + | ||
| 2787 | 2791 | // Extract repository from image reference, handling port numbers correctly. | |
| 2788 | 2792 | // Examples: | |
| 2789 | 2793 | // nginx:latest -> docker.io/library/nginx | |
| @@ -2784,6 +2784,10 @@ func (s *Service) resolveExistingAttachment(ctx context.Context, ownerDomain, se | |||||
| 2784 | // Utility functions | 2784 | // Utility functions | ||
| 2785 | 2785 | ||||
| 2786 | func normalizeImageRef(image string) string { | 2786 | func normalizeImageRef(image string) string { | ||
| 2787 | + | if isDigestRef(image) { | |||
| 2788 | + | return image | |||
| 2789 | + | } | |||
| 2790 | + | ||||
| 2787 | // Extract repository from image reference, handling port numbers correctly. | 2791 | // Extract repository from image reference, handling port numbers correctly. | ||
| 2788 | // Examples: | 2792 | // Examples: | ||
| 2789 | // nginx:latest -> docker.io/library/nginx | 2793 | // nginx:latest -> docker.io/library/nginx | ||
M internal/usecase/container/service_test.go +10 -0
| @@ -1675,6 +1675,16 @@ func TestNormalizeImageRef(t *testing.T) { | |||
| 1675 | 1675 | input: "registry.example.com:5000/image:v1.0", | |
| 1676 | 1676 | expected: "registry.example.com:5000/image", | |
| 1677 | 1677 | }, | |
| 1678 | + | { | |
| 1679 | + | name: "digest ref preserves digest", | |
| 1680 | + | input: "registry.example.com/image@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", | |
| 1681 | + | expected: "registry.example.com/image@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", | |
| 1682 | + | }, | |
| 1683 | + | { | |
| 1684 | + | name: "docker hub digest ref preserves digest", | |
| 1685 | + | input: "nginx@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", | |
| 1686 | + | expected: "nginx@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", | |
| 1687 | + | }, | |
| 1678 | 1688 | } | |
| 1679 | 1689 | ||
| 1680 | 1690 | for _, tt := range tests { | |
| @@ -1675,6 +1675,16 @@ func TestNormalizeImageRef(t *testing.T) { | |||||
| 1675 | input: "registry.example.com:5000/image:v1.0", | 1675 | input: "registry.example.com:5000/image:v1.0", | ||
| 1676 | expected: "registry.example.com:5000/image", | 1676 | expected: "registry.example.com:5000/image", | ||
| 1677 | }, | 1677 | }, | ||
| 1678 | + | { | |||
| 1679 | + | name: "digest ref preserves digest", | |||
| 1680 | + | input: "registry.example.com/image@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", | |||
| 1681 | + | expected: "registry.example.com/image@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", | |||
| 1682 | + | }, | |||
| 1683 | + | { | |||
| 1684 | + | name: "docker hub digest ref preserves digest", | |||
| 1685 | + | input: "nginx@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", | |||
| 1686 | + | expected: "nginx@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", | |||
| 1687 | + | }, | |||
| 1678 | } | 1688 | } | ||
| 1679 | 1689 | ||||
| 1680 | for _, tt := range tests { | 1690 | for _, tt := range tests { | ||