From dce42405d7aaa6b0e1aaa1e54ceab3e4a29f03be Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Mon, 9 Nov 2026 13:45:15 -0500 Subject: [PATCH] feat(sync): add source descriptor storage Add the .ref.yaml descriptor: its path for each resource kind, its validation, and its atomic create and replace. A resource has one managed file or one descriptor, never both. Co-Authored-By: Claude Fable 5.1 --- internal/sync/detach/detach_test.go | 9 +- internal/sync/local/create.go | 58 +++- internal/sync/local/delete.go | 84 ++++- internal/sync/local/generated.go | 72 ++++ internal/sync/local/layout.go | 60 ++++ internal/sync/local/reference.go | 458 ++++++++++++++++++++++++++ internal/sync/local/reference_test.go | 68 ++++ internal/sync/local/replace.go | 108 +++++- internal/sync/local/store.go | 48 ++- 9 files changed, 938 insertions(+), 27 deletions(-) create mode 100644 internal/sync/local/generated.go create mode 100644 internal/sync/local/reference.go create mode 100644 internal/sync/local/reference_test.go diff --git a/internal/sync/detach/detach_test.go b/internal/sync/detach/detach_test.go index 9c6aa7427..928fdb7f2 100644 --- a/internal/sync/detach/detach_test.go +++ b/internal/sync/detach/detach_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "slices" "strings" + "syscall" "testing" "github.com/stretchr/testify/assert" @@ -425,11 +426,11 @@ func TestRunKeepsEveryFileWhenOneDeleteFails(t *testing.T) { workspace := newArchiveWorkspace(t, "prompt") tool := testTool() workspace.addAttachments(t, tool) - // A symbolic link in place of the tool file makes its delete fail. + // A named pipe in place of the tool file is not a regular file, so its + // delete fails. toolFile := filepath.Join(workspace.root, syncdomain.RootDir, "project", "tools", "search.json") - target := filepath.Join(t.TempDir(), "search.json") - require.NoError(t, os.Rename(toolFile, target)) - require.NoError(t, os.Symlink(target, toolFile)) + require.NoError(t, os.Remove(toolFile)) + require.NoError(t, syscall.Mkfifo(toolFile, 0o644)) options := workspace.options(&recordingArchiver{}) options.Selections = append(options.Selections, tool.ID("project")) options.Yes = true diff --git a/internal/sync/local/create.go b/internal/sync/local/create.go index c6d78bfa2..13b03cb0f 100644 --- a/internal/sync/local/create.go +++ b/internal/sync/local/create.go @@ -4,8 +4,11 @@ import ( "errors" "fmt" "os" + "path" "path/filepath" "strings" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" ) // Creation records the result of a create. VariationPaths are the variation @@ -73,17 +76,54 @@ func (store Store) Bootstrap(files []VariationFile) (Creation, error) { } // Add creates a batch of variation files and the tool and skill files that -// they use. Add never overwrites a file. An existing variation file is an -// error, and an existing tool or skill file stays as it is, so that a local -// edit is not lost. If one file fails, Add removes the files that it created. +// they use. Add never overwrites a file. An existing variation file or +// descriptor is an error, and an existing tool or skill, as a managed file or +// as a descriptor, stays as it is, so that a local edit is not lost. If one +// file fails, Add removes the files that it created. func (store Store) Add(files []VariationFile) (Creation, error) { rendered, err := store.Render(files) if err != nil { return Creation{}, err } + linked := make(map[string]bool) + for _, file := range rendered { + id, ok := ParseManagedPath(path.Join(syncdomain.RootDir, file.Path)) + if !ok { + continue + } + repositoryReferencePath, err := ReferencePath(id) + if err != nil { + return Creation{}, err + } + referencePath := strings.TrimPrefix(repositoryReferencePath, syncdomain.RootDir+"/") + exists, err := store.fileExists(referencePath, id.Label()+" descriptor") + if err != nil { + return Creation{}, err + } + if !exists { + continue + } + if id.Kind == syncdomain.KindVariation { + return Creation{}, definitionConflict(id, id.Label()+" already has a descriptor") + } + // A tool or skill that a descriptor defines has that one definition. + // A managed file beside the descriptor is a conflict, not a file to + // keep. + managed, err := store.fileExists(file.Path, id.Label()) + if err != nil { + return Creation{}, err + } + if managed { + return Creation{}, definitionConflict(id, id.Label()+" has a managed file and a descriptor") + } + linked[file.Path] = true + } var creation Creation for _, file := range rendered { + if linked[file.Path] { + continue + } created, err := createFileIfAbsent(store.root, store.absolute(file.Path), file.Content) if err != nil { return Creation{}, errors.Join(err, store.RollbackCreation(creation)) @@ -116,8 +156,18 @@ func (store Store) RollbackCreation(creation Creation) error { // createFileIfAbsent creates one file and reports whether it did. An existing // variation file is an error. An existing tool or skill file is kept. func createFileIfAbsent(root, path string, data []byte) (bool, error) { - _, err := os.Stat(path) + info, err := os.Lstat(path) switch { + case err == nil && info.Mode()&os.ModeSymlink != 0: + // A leftover link at a managed path is not a definition, as it is + // not for compile, so the add replaces it with the file. The + // parents of the path must not be links. + if err := rejectSymlinkedPath(root, filepath.Dir(path)); err != nil { + return false, err + } + if err := os.Remove(path); err != nil { + return false, fmt.Errorf("remove leftover link %s: %w", filepath.Base(path), err) + } case err == nil: if err := rejectSymlinkedPath(root, path); err != nil { return false, err diff --git a/internal/sync/local/delete.go b/internal/sync/local/delete.go index bc6985153..80bece516 100644 --- a/internal/sync/local/delete.go +++ b/internal/sync/local/delete.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" syncdomain "github.com/launchdarkly/ldcli/internal/sync" ) @@ -44,6 +45,69 @@ func (store Store) DeleteResources(ids []syncdomain.ResourceID) ([]string, error return store.deleteFiles(paths) } +// DeleteDefinitions deletes each descriptor when present, or its managed file. +// It never deletes the external file that a descriptor references. +func (store Store) DeleteDefinitions(ids []syncdomain.ResourceID) ([]string, error) { + paths := make([]string, 0, len(ids)) + for _, id := range ids { + reference, err := ReferencePath(id) + if err != nil { + return nil, err + } + reference = strings.TrimPrefix(reference, syncdomain.RootDir+"/") + referenced, err := store.fileExists(reference, id.Label()+" descriptor") + if err != nil { + return nil, err + } + managed, err := resourcePath(id) + if err != nil { + return nil, err + } + // A leftover link at the managed path is not a definition, as it is + // not for compile, so the descriptor beside it is the one to delete. + managedExists, err := store.fileExists(managed, id.Label()) + if err != nil { + return nil, err + } + link, err := store.isLink(managed) + if err != nil { + return nil, err + } + switch { + case referenced && managedExists: + return nil, definitionConflict(id, id.Label()+" has a managed file and a descriptor") + case referenced: + paths = append(paths, reference) + // A leftover link beside the descriptor goes with it, in the + // same transaction, so that the link does not stand alone, where + // compile rejects it. + if link { + paths = append(paths, managed) + } + case managedExists, link: + // A lone leftover link goes too: sync owns the managed + // directory, and the delete removes the link, not its target. + paths = append(paths, managed) + default: + return nil, fmt.Errorf("inspect definition %s: %w", id.Label(), os.ErrNotExist) + } + } + return store.deleteFiles(paths) +} + +// isLink reports whether a managed path is a symbolic link. +func (store Store) isLink(relativePath string) (bool, error) { + info, err := os.Lstat(store.absolute(relativePath)) + switch { + case err == nil: + return info.Mode()&os.ModeSymlink != 0, nil + case errors.Is(err, os.ErrNotExist): + return false, nil + default: + return false, fmt.Errorf("inspect %s: %w", relativePath, err) + } +} + // deleteFiles deletes a batch of managed files in one transaction. It first // moves every file to a backup beside it. If a move fails, it moves the // files back. When every file has moved, it removes the backups. @@ -57,16 +121,24 @@ func (store Store) deleteFiles(relativePaths []string) ([]string, error) { seen[relativePath] = struct{}{} path := store.absolute(relativePath) - if err := rejectSymlinkedPath(store.root, path); err != nil { - return nil, err - } - // Lstat does not follow a symbolic link, so a link is not a regular file. + // Lstat does not follow a symbolic link, so a link is not a regular + // file. A link itself can go, as a rename moves the link and not its + // target, but no parent of a managed file is a link. info, err := os.Lstat(path) if err != nil { return nil, fmt.Errorf("inspect %s: %w", relativePath, err) } - if !info.Mode().IsRegular() { - return nil, fmt.Errorf("%s is not a regular file", relativePath) + if info.Mode()&os.ModeSymlink != 0 { + if err := rejectSymlinkedPath(store.root, filepath.Dir(path)); err != nil { + return nil, err + } + } else { + if err := rejectSymlinkedPath(store.root, path); err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("%s is not a regular file", relativePath) + } } deletions = append(deletions, stagedDeletion{relativePath: relativePath, path: path}) } diff --git a/internal/sync/local/generated.go b/internal/sync/local/generated.go new file mode 100644 index 000000000..bd7815d12 --- /dev/null +++ b/internal/sync/local/generated.go @@ -0,0 +1,72 @@ +package local + +import ( + "fmt" + "path/filepath" + "strings" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" +) + +// This file tells an authored source from a generated, vendored, or minified +// one, which sync does not edit. + +// publicationProblem is the problem of a source that sync cannot write. +func publicationProblem(file, message, suggestion string, cause error) error { + return syncdomain.WrapProblem( + syncdomain.NewProblem(syncdomain.ErrorSourceNotWritable, message, suggestion).WithFile(file, 0, 0), + cause, + ) +} + +// generatedSource reports whether a path or its content marks a file that a +// tool produced: a vendored or minified path, or a generated-code header. +func generatedSource(file string, data []byte) bool { + // A case-insensitive volume serves `Vendor/` and `.Min.js` too, as the + // managed-directory check allows for. + path := strings.ToLower(filepath.ToSlash(file)) + if strings.Contains("/"+path+"/", "/vendor/") || strings.HasSuffix(path, ".min.js") { + return true + } + prefix := strings.ToLower(string(data)) + if len(prefix) > 512 { + prefix = prefix[:512] + } + return strings.Contains(prefix, "code generated") && strings.Contains(prefix, "do not edit") +} + +// generatedTarget reports whether a source whose resolved path is target is +// generated, vendored, or minified. A link can point into such a tree, so the +// resolved path counts as well as the name in the descriptor, on every read. +func generatedTarget(repositoryRoot, file, target string, data []byte) (bool, error) { + // A root or a target that does not resolve hides whether the source is + // writable, so the check fails closed. + root, err := filepath.EvalSymlinks(repositoryRoot) + if err != nil { + return false, fmt.Errorf("resolve repository root: %w", err) + } + resolved, err := filepath.Rel(root, target) + if err != nil { + return false, fmt.Errorf("resolve source file %q: %w", file, err) + } + return generatedSource(file, data) || generatedSource(filepath.ToSlash(resolved), data), nil +} + +// rejectGeneratedTarget reports a source that sync must write but that is +// generated, vendored, or minified. +func rejectGeneratedTarget(repositoryRoot, file, target string, data []byte) error { + generated, err := generatedTarget(repositoryRoot, file, target, data) + if err != nil { + return err + } + if generated { + return generatedProblem(file) + } + return nil +} + +// generatedProblem is the problem of a source that sync must write but that +// a tool produced. +func generatedProblem(file string) error { + return publicationProblem(file, "source is generated, vendored, or minified", "Link the authored source file.", nil) +} diff --git a/internal/sync/local/layout.go b/internal/sync/local/layout.go index 8e86cb558..d4a3db184 100644 --- a/internal/sync/local/layout.go +++ b/internal/sync/local/layout.go @@ -26,6 +26,7 @@ const ( variationFileSuffix = ".prompt.md" toolFileSuffix = ".json" skillFileSuffix = ".md" + referenceFileSuffix = ".ref.yaml" ) // ParseManagedPath returns the resource that a managed file stores. The path @@ -62,6 +63,37 @@ func ParseManagedPath(file string) (syncdomain.ResourceID, bool) { return id, true } +// ParseReferencePath returns the resource that a .ref.yaml descriptor links. +func ParseReferencePath(file string) (syncdomain.ResourceID, bool) { + parts := strings.Split(file, "/") + if len(parts) < 4 || parts[0] != syncdomain.RootDir { + return syncdomain.ResourceID{}, false + } + + var id syncdomain.ResourceID + var stem string + switch { + case len(parts) == 5 && parts[2] == configsDir && strings.HasSuffix(parts[4], referenceFileSuffix): + stem = strings.TrimSuffix(parts[4], referenceFileSuffix) + if syncdomain.ValidateKey(parts[3]) != nil { + return syncdomain.ResourceID{}, false + } + id = syncdomain.VariationID(parts[1], parts[3], stem) + case len(parts) == 4 && parts[2] == toolsDir && strings.HasSuffix(parts[3], referenceFileSuffix): + stem = strings.TrimSuffix(parts[3], referenceFileSuffix) + id = syncdomain.ResourceID{Kind: syncdomain.KindTool, ProjectKey: parts[1], LookupKey: stem} + case len(parts) == 4 && parts[2] == skillsDir && strings.HasSuffix(parts[3], referenceFileSuffix): + stem = strings.TrimSuffix(parts[3], referenceFileSuffix) + id = syncdomain.ResourceID{Kind: syncdomain.KindSkill, ProjectKey: parts[1], LookupKey: stem} + default: + return syncdomain.ResourceID{}, false + } + if syncdomain.ValidateKey(parts[1]) != nil || syncdomain.ValidateKey(stem) != nil { + return syncdomain.ResourceID{}, false + } + return id, true +} + // ResourcePath returns the path of the file of a variation, a tool, or a // skill relative to the repository root, in the form that ParseManagedPath // reads. @@ -73,6 +105,34 @@ func ResourcePath(id syncdomain.ResourceID) (string, error) { return path.Join(syncdomain.RootDir, relativePath), nil } +// ReferencePath returns the repository-relative path of a resource descriptor. +func ReferencePath(id syncdomain.ResourceID) (string, error) { + if err := validateKey("project", id.ProjectKey); err != nil { + return "", err + } + if id.Kind == syncdomain.KindVariation { + configKey, variationKey, err := id.VariationKeys() + if err != nil { + return "", err + } + if err := cmp.Or(validateKey("config", configKey), validateKey("variation", variationKey)); err != nil { + return "", err + } + return path.Join(syncdomain.RootDir, id.ProjectKey, configsDir, configKey, variationKey+referenceFileSuffix), nil + } + if err := validateKey(string(id.Kind), id.LookupKey); err != nil { + return "", err + } + switch id.Kind { + case syncdomain.KindTool: + return path.Join(syncdomain.RootDir, id.ProjectKey, toolsDir, id.LookupKey+referenceFileSuffix), nil + case syncdomain.KindSkill: + return path.Join(syncdomain.RootDir, id.ProjectKey, skillsDir, id.LookupKey+referenceFileSuffix), nil + default: + return "", fmt.Errorf("unsupported resource kind %q", id.Kind) + } +} + // variationPath returns the path of a variation file relative to the managed // directory. It makes sure that each key is a safe path segment. func variationPath(projectKey, configKey, variationKey string) (string, error) { diff --git a/internal/sync/local/reference.go b/internal/sync/local/reference.go new file mode 100644 index 000000000..9488e600f --- /dev/null +++ b/internal/sync/local/reference.go @@ -0,0 +1,458 @@ +package local + +import ( + "bytes" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path" + "path/filepath" + "slices" + "strings" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" + syncadapter "github.com/launchdarkly/ldcli/internal/sync/adapter" + "gopkg.in/yaml.v3" +) + +// Reference identifies one repository source and its mapped fields. +type Reference = syncdomain.Reference + +func definitionConflict(id syncdomain.ResourceID, message string) error { + return syncdomain.NewProblem( + syncdomain.ErrorDefinitionConflict, + message, + "Keep one managed file or one .ref.yaml descriptor for this resource.", + ).WithResource(id) +} + +// NewSourceReference validates an external source and its mapping. +func NewSourceReference( + repositoryRoot, workingDirectory, file, format, selectPath string, + fieldMap map[string]string, +) (Reference, error) { + if file == "" { + return Reference{}, errors.New("linked file path is required") + } + if !filepath.IsAbs(file) { + file = filepath.Join(workingDirectory, file) + } + // Compare resolved paths. A path that looks like it is in the repository + // can go out of it through a symbolic link. + target, err := filepath.EvalSymlinks(file) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return Reference{}, syncdomain.NewProblem( + syncdomain.ErrorLinkedSourceMissing, + fmt.Sprintf("source file %q does not exist", file), + "Restore the file or pass another --file.", + ).WithFile(file, 0, 0) + } + return Reference{}, fmt.Errorf("resolve linked file %q: %w", file, err) + } + root, err := filepath.EvalSymlinks(repositoryRoot) + if err != nil { + return Reference{}, fmt.Errorf("resolve repository root: %w", err) + } + if !isWithin(root, target) { + return Reference{}, errors.New("linked file must be inside the Git repository") + } + + relative, _ := filepath.Rel(root, target) + reference := Reference{ + File: filepath.ToSlash(relative), Format: format, Select: selectPath, Map: fieldMap, + } + if _, err := resolveSourcePath(root, reference.File); err != nil { + return Reference{}, err + } + // A generated, vendored, or minified source is rejected on every read, + // so the link rejects it before it writes a descriptor. + data, err := os.ReadFile(target) + if err != nil { + return Reference{}, fmt.Errorf("read linked file %q: %w", file, err) + } + if err := rejectGeneratedTarget(repositoryRoot, reference.File, target, data); err != nil { + return Reference{}, err + } + return reference, nil +} + +const referenceFormatVersion = 1 + +type referenceDescriptor struct { + FormatVersion int `yaml:"formatVersion"` + Reference `yaml:",inline"` +} + +// ReadDescriptor reads and validates one .ref.yaml file. +func ReadDescriptor(fsys fs.FS, descriptorPath string, id syncdomain.ResourceID) (Reference, error) { + data, err := fs.ReadFile(fsys, descriptorPath) + if err != nil { + return Reference{}, err + } + return decodeDescriptor(data, id) +} + +// ParseDescriptor decodes and validates the content of one .ref.yaml file +// that the caller read, so that one read serves the content and the +// reference. +func ParseDescriptor(data []byte, id syncdomain.ResourceID) (Reference, error) { + return decodeDescriptor(data, id) +} + +// decodeDescriptor decodes and validates the content of one .ref.yaml file. +func decodeDescriptor(data []byte, id syncdomain.ResourceID) (Reference, error) { + var descriptor referenceDescriptor + decoder := yaml.NewDecoder(bytes.NewReader(data)) + decoder.KnownFields(true) + if err := decoder.Decode(&descriptor); err != nil { + return Reference{}, fmt.Errorf("decode descriptor: %w", err) + } + var extra any + if err := decoder.Decode(&extra); err != io.EOF { + if err == nil { + err = errors.New("more than one YAML document") + } + return Reference{}, fmt.Errorf("decode descriptor: %w", err) + } + if descriptor.FormatVersion != referenceFormatVersion { + return Reference{}, fmt.Errorf("unsupported formatVersion %d", descriptor.FormatVersion) + } + if err := validateDescriptorRelationships(id, descriptor.Reference); err != nil { + return Reference{}, err + } + if err := validateReference(descriptor.Reference); err != nil { + return Reference{}, err + } + reference, err := syncadapter.NormalizeReference(id, descriptor.Reference) + if err != nil { + return Reference{}, err + } + sortReferenceRelationships(&reference) + return reference, nil +} + +// RenderDescriptor encodes one editable .ref.yaml descriptor. It applies the +// same checks as ReadDescriptor, so a written descriptor always reads back. +func RenderDescriptor(id syncdomain.ResourceID, reference Reference) ([]byte, error) { + if err := validateDescriptorRelationships(id, reference); err != nil { + return nil, err + } + if err := validateReference(reference); err != nil { + return nil, err + } + reference, err := syncadapter.NormalizeReference(id, reference) + if err != nil { + return nil, err + } + sortReferenceRelationships(&reference) + var content bytes.Buffer + encoder := yaml.NewEncoder(&content) + encoder.SetIndent(2) + if err := encoder.Encode(referenceDescriptor{ + FormatVersion: referenceFormatVersion, + Reference: reference, + }); err != nil { + return nil, fmt.Errorf("encode descriptor: %w", err) + } + if err := encoder.Close(); err != nil { + return nil, fmt.Errorf("encode descriptor: %w", err) + } + return content.Bytes(), nil +} + +func validateDescriptorRelationships(id syncdomain.ResourceID, reference Reference) error { + if id.Kind != syncdomain.KindVariation && reference.Tools != nil { + return fmt.Errorf("top-level %q is not valid for %s descriptors", syncdomain.FieldTools, id.Kind) + } + if id.Kind != syncdomain.KindVariation && reference.Skills != nil { + return fmt.Errorf("top-level %q is not valid for %s descriptors", syncdomain.FieldSkills, id.Kind) + } + var tools, skills []syncdomain.AttachmentRef + if reference.Tools != nil { + tools = *reference.Tools + } + if reference.Skills != nil { + skills = *reference.Skills + } + return syncdomain.ValidateAttachmentRefs(syncdomain.VariationModeAgent, tools, skills) +} + +func sortReferenceRelationships(reference *Reference) { + sortRefs := func(refs *[]syncdomain.AttachmentRef) *[]syncdomain.AttachmentRef { + if refs == nil { + return nil + } + values := slices.Clone(*refs) + slices.SortFunc(values, func(left, right syncdomain.AttachmentRef) int { + return strings.Compare(left.Key, right.Key) + }) + return &values + } + reference.Tools = sortRefs(reference.Tools) + reference.Skills = sortRefs(reference.Skills) +} + +// ValidateDescriptorTarget rejects a managed definition of the same resource. +func (store Store) ValidateDescriptorTarget(id syncdomain.ResourceID) error { + managed, err := resourcePath(id) + if err != nil { + return err + } + if exists, err := store.fileExists(managed, id.Label()); err != nil { + return err + } else if exists { + return definitionConflict(id, id.Label()+" already has a managed file") + } + return nil +} + +// WriteDescriptor creates or replaces one resource descriptor atomically. +func (store Store) WriteDescriptor(id syncdomain.ResourceID, reference Reference) (string, error) { + if err := store.ValidateDescriptorTarget(id); err != nil { + return "", err + } + + repositoryPath, err := ReferencePath(id) + if err != nil { + return "", err + } + relativePath := strings.TrimPrefix(repositoryPath, syncdomain.RootDir+"/") + target := store.absolute(relativePath) + content, err := RenderDescriptor(id, reference) + if err != nil { + return "", err + } + // Sync owns the managed directory, so a create and a replace both refuse + // a path that goes through a symbolic link. + if err := rejectSymlinkedPath(store.root, target); err != nil { + return "", err + } + current, err := os.ReadFile(target) + switch { + case errors.Is(err, os.ErrNotExist): + if err := createFile(store.root, target, content); err != nil { + return "", err + } + case err != nil: + return "", fmt.Errorf("read descriptor %q: %w", repositoryPath, err) + case bytes.Equal(current, content): + return repositoryPath, nil + default: + info, err := os.Stat(target) + if err != nil { + return "", fmt.Errorf("inspect descriptor %q: %w", repositoryPath, err) + } + if err := ReplaceFileAtomically(target, repositoryPath, current, content, info.Mode().Perm()); err != nil { + return "", err + } + } + return repositoryPath, nil +} + +// WriteDescriptorFrom creates or replaces one resource descriptor, when the +// descriptor still has the content previous, or does not exist when previous +// is nil. A descriptor that another process wrote in between is a problem, +// so a reviewed link does not write over it. +func (store Store) WriteDescriptorFrom(id syncdomain.ResourceID, reference Reference, previous []byte) (string, error) { + if err := store.ValidateDescriptorTarget(id); err != nil { + return "", err + } + repositoryPath, err := ReferencePath(id) + if err != nil { + return "", err + } + target := store.absolute(strings.TrimPrefix(repositoryPath, syncdomain.RootDir+"/")) + content, err := RenderDescriptor(id, reference) + if err != nil { + return "", err + } + if err := rejectSymlinkedPath(store.root, target); err != nil { + return "", err + } + changed := syncdomain.NewProblem( + syncdomain.ErrorStateChanged, "the descriptor changed after review", "Run link again.", + ).WithResource(id).WithFile(repositoryPath, 0, 0) + if len(previous) == 0 { + // The create refuses a file that another process created since. + if err := createFile(store.root, target, content); err != nil { + if errors.Is(err, ErrVariationExists) { + return "", changed + } + return "", err + } + return repositoryPath, nil + } + current, err := os.ReadFile(target) + switch { + case errors.Is(err, os.ErrNotExist): + return "", changed + case err != nil: + return "", fmt.Errorf("read descriptor %q: %w", repositoryPath, err) + case !bytes.Equal(current, previous): + return "", changed + case bytes.Equal(current, content): + return repositoryPath, nil + } + info, err := os.Stat(target) + if err != nil { + return "", fmt.Errorf("inspect descriptor %q: %w", repositoryPath, err) + } + // The replace checks the content again right before the rename. + if err := ReplaceFileAtomically(target, repositoryPath, previous, content, info.Mode().Perm()); err != nil { + return "", err + } + return repositoryPath, nil +} + +// RestoreDescriptor puts the previous content of a descriptor back, or +// removes the descriptor when it did not exist, when the descriptor still has +// the content written. A link that cannot finish uses it, so that the next +// sync does not see a binding that was never recorded. A descriptor that +// another process wrote since stays. +func (store Store) RestoreDescriptor(id syncdomain.ResourceID, previous, written []byte) error { + repositoryPath, err := ReferencePath(id) + if err != nil { + return err + } + relativePath := strings.TrimPrefix(repositoryPath, syncdomain.RootDir+"/") + target := store.absolute(relativePath) + if err := rejectSymlinkedPath(store.root, target); err != nil { + return err + } + current, err := os.ReadFile(target) + switch { + case errors.Is(err, os.ErrNotExist): + // A link that failed before the write left nothing to remove, and + // that is the wanted state. + return nil + case err != nil: + return fmt.Errorf("read descriptor %q: %w", repositoryPath, err) + case bytes.Equal(current, previous): + return nil + case !bytes.Equal(current, written): + // Another process wrote the descriptor, so it is not this link's + // to remove or to replace. + return nil + } + if len(previous) == 0 { + // The link created the descriptor. + _, err := store.deleteFiles([]string{relativePath}) + return err + } + info, err := os.Stat(target) + if err != nil { + return fmt.Errorf("inspect descriptor %q: %w", repositoryPath, err) + } + return ReplaceFileAtomically(target, repositoryPath, current, previous, info.Mode().Perm()) +} + +// validateReference makes sure that the descriptor names a format and a +// clean repository-relative file outside the managed directory. +func validateReference(reference Reference) error { + if reference.Format == "" { + return errors.New("ref.format is required") + } + return validateSourcePath(reference.File) +} + +// validateSourcePath makes sure that file is a clean path in the repository +// and outside the managed directory. +func validateSourcePath(file string) error { + switch { + case file == "": + return errors.New("ref.file is required") + case !fs.ValidPath(file) || strings.Contains(file, "\\") || filepath.VolumeName(file) != "" || + len(file) > 1 && file[1] == ':': + return fmt.Errorf("ref.file %q must be a repository-relative path", file) + case inManagedDirectory(file): + return fmt.Errorf("ref.file %q must be outside %s", file, syncdomain.RootDir) + case path.Clean(file) != file: + return fmt.Errorf("ref.file %q must be a clean repository-relative path", file) + default: + return nil + } +} + +// inManagedDirectory reports whether a repository-relative path is the +// managed directory or inside it. The comparison ignores case, because a +// case-insensitive volume opens .LaunchDarkly as the managed directory. +func inManagedDirectory(file string) bool { + first, _, _ := strings.Cut(filepath.ToSlash(file), "/") + return strings.EqualFold(first, syncdomain.RootDir) +} + +// resolveSourcePath returns the absolute path of a repository source file. It +// resolves symbolic links on each call, because a link can change after the +// resource was linked. +func resolveSourcePath(repositoryRoot, file string) (string, error) { + if err := validateSourcePath(file); err != nil { + return "", err + } + root, err := filepath.EvalSymlinks(repositoryRoot) + if err != nil { + return "", fmt.Errorf("resolve repository root: %w", err) + } + target, err := filepath.EvalSymlinks(filepath.Join(root, filepath.FromSlash(file))) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return "", syncdomain.NewProblem( + syncdomain.ErrorLinkedSourceMissing, + fmt.Sprintf("source file %q does not exist", file), + "Restore the file or run detach.", + ).WithFile(file, 0, 0) + } + return "", fmt.Errorf("resolve referenced file %q: %w", file, err) + } + if !isWithin(root, target) { + return "", fmt.Errorf("referenced file %q resolves outside the Git repository", file) + } + // A link can point into the managed directory, so check the resolved + // path as well as the stored one. + if relative, err := filepath.Rel(root, target); err != nil || inManagedDirectory(relative) { + return "", fmt.Errorf("referenced file %q resolves inside %s", file, syncdomain.RootDir) + } + info, err := os.Stat(target) + if err != nil { + return "", fmt.Errorf("inspect referenced file %q: %w", file, err) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("referenced file %q is not a regular file", file) + } + return target, nil +} + +// ReadSourceFile reads one repository source file that sync can write. It +// resolves symbolic links and makes sure that the file is a regular file in +// the repository. +func ReadSourceFile(repositoryRoot, file string) ([]byte, error) { + document, err := readSourceDocument(repositoryRoot, file) + if err != nil { + return nil, err + } + if document.ReadOnly { + return nil, generatedProblem(file) + } + return document.Data, nil +} + +// readSourceDocument reads one repository source file. A generated, vendored, +// or minified file reads as read-only: sync follows an import into it, but +// never writes it. +func readSourceDocument(repositoryRoot, file string) (syncadapter.Document, error) { + target, err := resolveSourcePath(repositoryRoot, file) + if err != nil { + return syncadapter.Document{}, err + } + data, err := os.ReadFile(target) + if err != nil { + return syncadapter.Document{}, fmt.Errorf("read referenced file %q: %w", file, err) + } + generated, err := generatedTarget(repositoryRoot, file, target, data) + if err != nil { + return syncadapter.Document{}, err + } + return syncadapter.Document{Path: file, Data: data, ReadOnly: generated}, nil +} diff --git a/internal/sync/local/reference_test.go b/internal/sync/local/reference_test.go new file mode 100644 index 000000000..37002068a --- /dev/null +++ b/internal/sync/local/reference_test.go @@ -0,0 +1,68 @@ +package local + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" +) + +func TestNewSourceReferenceStoresRepositoryRelativePath(t *testing.T) { + root := t.TempDir() + workingDirectory := filepath.Join(root, "app") + require.NoError(t, os.MkdirAll(filepath.Join(workingDirectory, "prompts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(workingDirectory, "prompts", "support.md"), []byte("Help"), 0o644)) + + reference, err := NewSourceReference(root, workingDirectory, "prompts/support.md", "markdown", "$", nil) + + require.NoError(t, err) + require.Equal(t, "app/prompts/support.md", reference.File) +} + +func TestNewSourceReferenceRejectsUnsafePaths(t *testing.T) { + root := t.TempDir() + outside := filepath.Join(t.TempDir(), "prompt.md") + require.NoError(t, os.WriteFile(outside, []byte("Help"), 0o644)) + + _, err := NewSourceReference(root, root, outside, "markdown", "$", nil) + require.ErrorContains(t, err, "inside the Git repository") + + managed := filepath.Join(root, syncdomain.RootDir, "prompt.md") + require.NoError(t, os.MkdirAll(filepath.Dir(managed), 0o755)) + require.NoError(t, os.WriteFile(managed, []byte("Help"), 0o644)) + _, err = NewSourceReference(root, root, managed, "markdown", "$", nil) + require.ErrorContains(t, err, "must be outside") +} + +func TestNewSourceReferenceRejectsGeneratedSources(t *testing.T) { + root := t.TempDir() + vendored := filepath.Join(root, "vendor", "lib", "prompt.md") + require.NoError(t, os.MkdirAll(filepath.Dir(vendored), 0o755)) + require.NoError(t, os.WriteFile(vendored, []byte("Help"), 0o644)) + generated := filepath.Join(root, "prompts.go") + require.NoError(t, os.WriteFile(generated, []byte("// Code generated by tool. DO NOT EDIT.\npackage prompts\n"), 0o644)) + authored := filepath.Join(root, "prompt.md") + require.NoError(t, os.WriteFile(authored, []byte("Help"), 0o644)) + + _, err := NewSourceReference(root, root, vendored, "markdown", "$", nil) + require.ErrorContains(t, err, "generated, vendored, or minified") + _, err = NewSourceReference(root, root, generated, "markdown", "$", nil) + require.ErrorContains(t, err, "generated, vendored, or minified") + _, err = NewSourceReference(root, root, authored, "markdown", "$", nil) + require.NoError(t, err) +} + +func TestNewSourceReferenceRejectsSymlinkEscape(t *testing.T) { + root := t.TempDir() + outside := filepath.Join(t.TempDir(), "prompt.md") + require.NoError(t, os.WriteFile(outside, []byte("Help"), 0o644)) + link := filepath.Join(root, "prompt.md") + require.NoError(t, os.Symlink(outside, link)) + + _, err := NewSourceReference(root, root, link, "markdown", "$", nil) + + require.ErrorContains(t, err, "inside the Git repository") +} diff --git a/internal/sync/local/replace.go b/internal/sync/local/replace.go index 6d3146c14..4f720edff 100644 --- a/internal/sync/local/replace.go +++ b/internal/sync/local/replace.go @@ -5,7 +5,9 @@ import ( "errors" "fmt" "os" + "path" "path/filepath" + "strings" syncdomain "github.com/launchdarkly/ldcli/internal/sync" ) @@ -21,6 +23,16 @@ type stagedFile struct { mode os.FileMode stagedPath string isVariationFile bool + // verifyOnly marks a file that the transaction reads but does not + // write: the commit makes sure that it still has its original content. + verifyOnly bool + // sources holds each repository-relative name that a descriptor gives a + // source file, when the file is one, with root the repository root. The + // commit makes sure that every name still resolves to path, so that a + // link retargeted after the snapshot does not get the edit of another + // file. + sources []string + root string } // ReplaceVariations replaces a batch of variation files in one transaction. @@ -80,6 +92,13 @@ func (store Store) ReplaceAttachments(attachments []syncdomain.SyncedAttachment) // file to a temporary path, makes sure that no original changed, and then // renames each file into place. func replaceFiles(files []stagedFile) error { + seen := make(map[string]struct{}, len(files)) + for _, file := range files { + if _, duplicate := seen[file.path]; duplicate { + return fmt.Errorf("file %q is staged more than once", file.displayPath) + } + seen[file.path] = struct{}{} + } if err := stageFiles(files); err != nil { return err } @@ -130,8 +149,26 @@ func (store Store) prepareReplacements(replacements []VariationReplacement) ([]s attachmentSources := make([]VariationFile, 0, len(replacements)) for _, replacement := range replacements { existing, err := store.readVariation(replacement.ProjectKey, replacement.ConfigKey, replacement.Variation.Key) - if replacement.CreateIfMissing && errors.Is(err, os.ErrNotExist) { + id := syncdomain.VariationID(replacement.ProjectKey, replacement.ConfigKey, replacement.Variation.Key) + managedPath, pathErr := resourcePath(id) + if pathErr != nil { + return nil, pathErr + } + linked, linkedErr := store.hasDescriptor(managedPath) + if linkedErr != nil { + return nil, linkedErr + } + switch { + case replacement.CreateIfMissing && errors.Is(err, os.ErrNotExist) && linked: + // A variation that a descriptor defines has that one definition. + // A managed file beside it would be a second one. + return nil, definitionConflict(id, id.Label()+" already has a descriptor") + case replacement.CreateIfMissing && errors.Is(err, os.ErrNotExist): existing, err = missingVariation(replacement) + case err == nil && linked: + // A managed file beside a descriptor is a conflict, so the + // replace does not keep two definitions that drift apart. + return nil, definitionConflict(id, id.Label()+" has a managed file and a descriptor") } if err != nil { return nil, err @@ -195,6 +232,16 @@ func (store Store) prepareAttachmentFiles(rendered []RenderedFile) ([]stagedFile var staged []stagedFile for _, file := range rendered { path := store.absolute(file.Path) + if info, err := os.Lstat(path); err == nil && info.Mode()&os.ModeSymlink != 0 { + // A leftover link beside a descriptor is not a definition, so + // the descriptor keeps the one definition. A link alone is + // rejected below. + if linked, err := store.hasDescriptor(file.Path); err != nil { + return nil, err + } else if linked { + continue + } + } if err := rejectSymlinkedPath(store.root, path); err != nil { return nil, err } @@ -203,9 +250,21 @@ func (store Store) prepareAttachmentFiles(rendered []RenderedFile) ([]stagedFile original, err := os.ReadFile(path) switch { case errors.Is(err, os.ErrNotExist): + // A tool or skill that a descriptor defines keeps that one + // definition. A second, managed one would be a conflict. + if linked, linkedErr := store.hasDescriptor(file.Path); linkedErr != nil { + return nil, linkedErr + } else if linked { + continue + } case err != nil: return nil, fmt.Errorf("read attachment %q: %w", file.Path, err) default: + // A managed file beside a descriptor is a conflict, so the + // replace does not keep two definitions that drift apart. + if err := store.rejectDescriptorBeside(file.Path); err != nil { + return nil, err + } info, err := os.Stat(path) if err != nil { return nil, fmt.Errorf("inspect attachment %q: %w", file.Path, err) @@ -223,9 +282,37 @@ func (store Store) prepareAttachmentFiles(rendered []RenderedFile) ([]stagedFile return staged, nil } +// hasDescriptor reports whether the resource of a managed path has a +// descriptor in the store. +func (store Store) hasDescriptor(managedPath string) (bool, error) { + id, ok := ParseManagedPath(path.Join(syncdomain.RootDir, managedPath)) + if !ok { + return false, nil + } + referencePath, err := ReferencePath(id) + if err != nil { + return false, err + } + return store.fileExists(strings.TrimPrefix(referencePath, syncdomain.RootDir+"/"), id.Label()+" descriptor") +} + +// rejectDescriptorBeside returns the definition conflict of a managed file +// that has a descriptor beside it. +func (store Store) rejectDescriptorBeside(managedPath string) error { + linked, err := store.hasDescriptor(managedPath) + if err != nil || !linked { + return err + } + id, _ := ParseManagedPath(path.Join(syncdomain.RootDir, managedPath)) + return definitionConflict(id, id.Label()+" has a managed file and a descriptor") +} + // stageFiles writes each replacement to a temporary file beside its destination. func stageFiles(files []stagedFile) error { for index := range files { + if files[index].verifyOnly { + continue + } if err := os.MkdirAll(filepath.Dir(files[index].path), 0o755); err != nil { removeStagedFiles(files) return fmt.Errorf("create resource directory %s: %w", files[index].displayPath, err) @@ -244,6 +331,19 @@ func stageFiles(files []stagedFile) error { // it. An editor can save a file while sync waits for review. func verifyOriginals(files []stagedFile) error { for _, file := range files { + for _, source := range file.sources { + resolved, err := resolveSourcePath(file.root, source) + if err != nil || resolved != file.path { + return fmt.Errorf("source %s changed while syncing: the name resolves to another file", source) + } + } + if !file.originalExists { + // A create replaces a leftover link at the path, which is not a + // definition, so the content behind the link is not an edit. + if info, err := os.Lstat(file.path); err == nil && info.Mode()&os.ModeSymlink != 0 { + continue + } + } current, err := os.ReadFile(file.path) if !file.originalExists && errors.Is(err, os.ErrNotExist) { continue @@ -266,6 +366,9 @@ func commitFiles(files []stagedFile) error { func commitFilesWithRename(files []stagedFile, rename func(string, string) error) error { for index, file := range files { + if file.verifyOnly { + continue + } if err := rename(file.stagedPath, file.path); err != nil { return errors.Join(fmt.Errorf("replace variation %s: %w", file.displayPath, err), restoreFiles(files[:index])) } @@ -278,6 +381,9 @@ func restoreFiles(files []stagedFile) error { var failures []error for index := len(files) - 1; index >= 0; index-- { file := files[index] + if file.verifyOnly { + continue + } if !file.originalExists { if err := os.Remove(file.path); err != nil && !errors.Is(err, os.ErrNotExist) { failures = append(failures, fmt.Errorf("roll back variation %s: %w", file.displayPath, err)) diff --git a/internal/sync/local/store.go b/internal/sync/local/store.go index 03a2c54cb..5104b8b35 100644 --- a/internal/sync/local/store.go +++ b/internal/sync/local/store.go @@ -75,30 +75,50 @@ func (store Store) Exists() (bool, error) { // VariationExists reports whether a variation file exists. func (store Store) VariationExists(projectKey, configKey, variationKey string) (bool, error) { - relativePath, err := variationPath(projectKey, configKey, variationKey) - if err != nil { - return false, err - } - return store.fileExists(relativePath, "variation "+variationKey) + return store.definitionExists( + syncdomain.VariationID(projectKey, configKey, variationKey), + "variation "+variationKey, + ) } // ResourceExists reports whether the file of a variation, a tool, or a skill // exists. func (store Store) ResourceExists(id syncdomain.ResourceID) (bool, error) { - relativePath, err := resourcePath(id) + return store.definitionExists(id, id.Label()) +} + +func (store Store) definitionExists(id syncdomain.ResourceID, label string) (bool, error) { + managedPath, err := resourcePath(id) + if err != nil { + return false, err + } + managed, err := store.fileExists(managedPath, label) if err != nil { return false, err } - return store.fileExists(relativePath, id.Label()) + repositoryReferencePath, err := ReferencePath(id) + if err != nil { + return false, err + } + referencePath := strings.TrimPrefix(repositoryReferencePath, syncdomain.RootDir+"/") + referenced, err := store.fileExists(referencePath, label+" descriptor") + if err != nil { + return false, err + } + if managed && referenced { + return false, definitionConflict(id, id.Label()+" has a managed file and a descriptor") + } + return managed || referenced, nil } -// fileExists reports whether a managed file exists. label names the file in -// an error. +// fileExists reports whether a managed file exists. A symbolic link is not a +// definition, as it is not for compile, so it does not exist here. label +// names the file in an error. func (store Store) fileExists(relativePath, label string) (bool, error) { - _, err := os.Stat(store.absolute(relativePath)) + info, err := os.Lstat(store.absolute(relativePath)) switch { case err == nil: - return true, nil + return info.Mode()&os.ModeSymlink == 0, nil case errors.Is(err, os.ErrNotExist): return false, nil default: @@ -157,7 +177,11 @@ func (store Store) readVariation(projectKey, configKey, variationKey string) (ex } absolutePath := store.absolute(relativePath) - info, err := os.Stat(absolutePath) + info, err := os.Lstat(absolutePath) + if err == nil && info.Mode()&os.ModeSymlink != 0 { + // A symbolic link is not a definition, as it is not for compile. + err = os.ErrNotExist + } if err != nil { return existingVariation{}, fmt.Errorf("inspect variation %s: %w", variationKey, err) }