diff --git a/cmd/kosli/snapshotS3.go b/cmd/kosli/snapshotS3.go index 41ae7f896..975c4a2f8 100644 --- a/cmd/kosli/snapshotS3.go +++ b/cmd/kosli/snapshotS3.go @@ -15,7 +15,8 @@ const snapshotS3ShortDesc = `Report a snapshot of the content of an AWS S3 bucke const snapshotS3LongDesc = snapshotS3ShortDesc + awsAuthDesc + ` You can report the entire bucket content, or filter some of the content using ^--include^ / ^--exclude^ (literal prefix match) or ^--include-regex^ / ^--exclude-regex^ (Go regular expressions matched against the full object key). In all cases, the content is reported as one artifact. If you wish to report separate files/dirs within the same bucket as separate artifacts, you need to run the command twice. -Object keys that cannot be stored as a local file, such as keys containing a ^..^ path segment, are rejected and fail the snapshot, naming the key. Two keys that resolve to the same local file are also an error. A legitimate key of that shape can be left out with ^--exclude-regex^ (anchor and escape it, since the pattern is a regular expression matched against the whole key); when ^--include^ or ^--include-regex^ is set, exclude filters are ignored, so narrow the include filter instead. +Object keys are never used as local file names: each object is downloaded to a temporary file, hashed and removed, and the fingerprint is computed from the keys and the content digests, so any key S3 accepts can be fingerprinted on any operating system. +Keys that cannot form a directory tree are rejected and fail the snapshot, naming every key involved: a key containing a ^..^ segment, two keys that resolve to the same path (such as ^a//b^ and ^a/b^), or an object whose key is also a prefix of other objects (such as ^a^ beside ^a/b^). A legitimate key of that shape can be left out with ^--exclude-regex^ (anchor and escape it, since the pattern is a regular expression matched against the whole key); when ^--include^ or ^--include-regex^ is set, exclude filters are ignored, so narrow the include filter instead. ` + kosliIgnoreDescNoExclude diff --git a/docs/adr/20260911-s3-fingerprint-from-virtual-tree.md b/docs/adr/20260911-s3-fingerprint-from-virtual-tree.md new file mode 100644 index 000000000..f5ed44e3e --- /dev/null +++ b/docs/adr/20260911-s3-fingerprint-from-virtual-tree.md @@ -0,0 +1,67 @@ +--- +title: "20260911 - Fingerprint S3 buckets from a virtual tree; object keys never become local paths" +description: "Download each object to an anonymous temp file, hash it, delete it, and compute the directory fingerprint from (key, sha256) pairs so that no S3 key is ever used as a filename" +status: "Proposed" +date: "2026-09-11" +--- + +# 20260911 - Fingerprint S3 buckets from a virtual tree; object keys never become local paths + +## Overview + +`kosli snapshot s3` stops recreating the bucket as a directory on the operator's machine. Each object is downloaded to an anonymous temp file, hashed, and deleted. The fingerprint is computed by `digest.VirtualDirSha256` from `(key, sha256)` pairs, which reproduces `digest.DirSha256` byte for byte. Content mode and the metadata mode of #1069 become one pipeline that differs only in where each object's sha256 comes from. + +## Context + +Today `getS3DataFromClient` writes every object to `filepath.Join(tempDir, key)` and fingerprints the resulting tree with `DirSha256`. The object key, which anyone with write access to the bucket controls, therefore names a file on the operator's filesystem. + +PR #1155 fenced that key after a privately reported traversal: it rejects `..` segments, opens each destination with `O_EXCL` so two keys cannot share a file, and turns `ENOTDIR` into a readable error. It closes the reported hole, but the key still shapes a path, so a class of problems remains: + +- Two directories differing only in case merge on macOS and Windows. Unicode normalisation on macOS does the same. Both are fingerprint collisions between distinct bucket contents. +- Legitimate keys fail: `...` and `.. ` directories everywhere, and on Windows also `CON`, any colon, and a leading backslash, purely because Windows would misread the name on disk. +- A key component over 255 bytes fails with a bare `ENAMETOOLONG`. +- The fingerprint depends on the platform the snapshot runs on. A bucket with `A/x` and `a/y`, or with `d\e.txt`, fingerprints differently on Linux than on macOS or Windows. +- Around sixty lines of production code and most of the #1155 test table reason about Windows and macOS filesystem semantics that CI never executes. + +Separately, #1069 (`--fingerprint-source metadata`) built `VirtualDirSha256` to reproduce `DirSha256` without a filesystem, and its key rule (`validateVirtualPath`, strict `path.Clean` equality) disagrees with #1155's on-disk rule in both directions: `a//b`, `./c.txt` and `/lead.txt` are accepted on disk and rejected virtually; `...` is rejected on disk and accepted virtually. A bucket that snapshots today would break when the default flips. + +The fingerprint format itself is fixed. An S3 snapshot must match the fingerprint of the deployed directory attested earlier with `kosli attest artifact --artifact-type dir`, and every existing environment snapshot on the server was computed as `DirSha256` of the tree the key layout produced. So the change must preserve, for every bucket that snapshots successfully today, the identical fingerprint and artifact name. + +## Decision + +1. **The key never touches the filesystem.** Each object is downloaded to `os.CreateTemp(tempDir, "object-*")`, hashed with `FileSha256`, closed and removed. Peak disk drops from the bucket size to the in-flight objects. The transfer manager keeps working unchanged because `*os.File` is an `io.WriterAt`, and so does `FakeS3Client`. + +2. **The key becomes a virtual path by one rule, shared by both modes.** A key holding a `..` segment is rejected first, on the raw key, so `a/../b` cannot fold silently onto `b`. The rest is `path.Clean(strings.TrimLeft(key, "/"))`, rejecting a result of `.`. The fold is exactly what `filepath.Join` produced on Linux, so `a//b`, `./c.txt` and `/lead.txt` land where they do today. Everything else, including `...`, `.. `, `CON`, colons and backslashes, is an ordinary name because nothing is created under it. None of this is for safety. `DirSha256` walks real directories, which can never hold an entry named `.`, `..` or the empty string, so the rule keeps virtual fingerprints inside the space an attested directory can match, and preserves the fingerprints existing buckets already have. + +3. **Collisions are data errors that name every key involved.** Two keys resolving to one path, and a key under a prefix that is also an object, cannot be represented as a tree. They are detected before the digest package sees them and reported together, bounded by `maxReportedS3KeyProblems` in both directions, at most ten problem lines each naming at most ten keys, with the existing advice to use `--exclude-regex` or narrow the include filter. + +4. **A root `.kosli_ignore` is honoured virtually.** Its rules are parsed by `digest.ParseIgnoreRules`, the same reading `DirSha256` gives the file, and resolved by `digest/virtualglob.go`, which reproduces `filepathx.Glob`, `filepath.Glob` and `filepath.Walk` step for step over the virtual tree rather than reimplementing what the globs appear to mean. That is what keeps their quirks identical: a literal `**/x` finds a root `x` spelled with a double slash, which the walk's cleaned paths never equal, so a root file `x` survives while a root directory `x` keeps its name and loses its contents; `**/*.log` is rebuilt cleaned and matches outright; and excluding `logs/*` leaves an empty directory whose name is still hashed. Exclusion therefore runs inside the tree walk, not by filtering the file list. The ignore file can never exclude itself, as in `DirSha256`. `digest.FilesNeedingContent` shares that walk so excluded objects are not downloaded at all, and `VirtualDirSha256` refuses a tree that needs a digest it was not given, so a skipped download can never leak into a fingerprint. Equivalence with `DirSha256` on a materialised tree is asserted for every rule shape in `TestVirtualIgnoreTestSuite`. + +5. **Downloads may run in parallel**, behind a fixed worker pool and a bytes-in-flight budget derived from listing sizes, with results written by index for determinism and a cancellable context so the first transport error stops in-flight multipart downloads. This is a performance property, not a safety one, and is delivered separately from the change this record describes; see #1167. Until it lands, downloads are sequential, exactly as before, and peak temp disk is one object. + +6. **The switch is pinned, not argued.** `TestPinnedFingerprints` in `internal/aws` holds fingerprints of representative fake buckets recorded against the key-layout implementation before it was replaced: unusual key shapes, a `.` sorting before a `/`, nested prefixes with folder markers, and a single object with its basename as artifact name. It was green before the switch and must stay green after it, alongside `TestGetS3DataFromClientKeepsTodaysLayoutForUnusualKeys` from #1155. + +## Guarantees + +The attest side is unchanged: `kosli attest artifact --artifact-type dir` still runs `DirSha256` on a real directory, and that defines the format. The snapshot side re-derives the same value from the bucket. Three guarantees follow, each with the test that holds it: + +- **Snapshot equals attestation.** `VirtualDirSha256` of the bucket's `(path, sha256)` pairs equals `DirSha256` of the directory those objects were uploaded from, and a single object equals `FileSha256` plus its basename. Held by the materialise-then-compare tests in `internal/digest` (`TestVirtualDirTestSuite`, `TestVirtualIgnoreTestSuite`) and by `TestMatchesAttestedDirectory` in `internal/aws`, which hashes a directory, uploads its files to the fake bucket and snapshots it. +- **Snapshot equals its own history.** Every bucket that snapshotted successfully under the key-layout implementation keeps its fingerprint and artifact name, because the fold rule is what `filepath.Join` did. Held by `TestPinnedFingerprints` and `TestGetS3DataFromClientKeepsTodaysLayoutForUnusualKeys`. +- **No object is lost silently.** Every key S3 accepts is representable, whatever the operating system, because the key never becomes a path. Rejections are of two kinds, and each fails the snapshot rather than shorten the manifest. Keys that cannot form a directory tree at all: a `..` segment, two keys folding onto one path, and an object that is also a prefix. S3 permits those, no filesystem does, so no attested directory could match them, and the error names every key involved. And listings the store returned malformed: an entry with no key, a key listed more than once, and no modification time on any matching object. Real S3 produces none of these; an S3-compatible store might, and each is an error rather than a guess. A path deeper than `globSeparatorsLimit` segments is also rejected, which no S3 key can reach, so the exported digest functions carry the bound for other callers. Held by `TestEveryS3KeyIsRepresentable`, which generates keys over S3's full character set, and `TestDownloadsExactlyTheContributingObjects`, which asserts the downloaded set equals the listed objects minus markers and exclusions. + +## Alternatives considered + +- **Keep #1155's fenced layout and add rules for the remaining cases.** Each remaining case (case folding, Unicode normalisation, component length) needs another platform-specific rule and none can be exercised in Linux CI. The layout is the cause, so fencing it further does not converge. +- **Temp files alone, still hashing the on-disk tree.** Not viable: `DirSha256` hashes every entry's basename, so renaming files changes the fingerprint. +- **A CRC64- or ETag-derived fingerprint to avoid downloading.** S3 stores a full-object CRC64NVME for every object uploaded since December 2024, which makes it tempting. Rejected: CRC64 is linear and trivially forgeable, so an attacker who can write the bucket can replace approved content while keeping the fingerprint, which defeats the purpose of a compliance fingerprint. Hashing the CRC with SHA256 adds no resistance. ETag is MD5 of parts for multipart uploads and depends on client chunk size. +- **Incremental snapshots keyed on stored checksums.** Only `VersionId` is a trustworthy skip signal and only with versioning enabled; the design needs persisted state the stateless CLI does not have. Left for a separate decision. + +## Consequences + +- The fingerprint is the same on every operating system, with Linux semantics. A snapshot run on Windows or macOS against a bucket with case-colliding or backslash keys moves to the Linux value. The same holds for a backslash inside an ignore rule: the virtual glob reads it as an escape, as `filepath.Glob` does on Linux, where on Windows `filepath.Glob` reads it as a separator, so such a rule now resolves differently for an S3 snapshot than for `attest artifact --artifact-type dir` run on the same Windows machine. This is a correction and is release-noted. +- `localPathForS3Key`, `filepath.IsLocal`, the `O_EXCL` open, the `ENOTDIR` branch, `containsSingleFile` and the platform-conditional tests from #1155 are deleted. No code in the S3 path branches on the operating system any more. The codebase does not get smaller, though: the virtual tree, the key rule with its collision reporting, and above all the faithful simulation of `filepathx.Glob` add several hundred lines, most of them owed to reproducing `.kosli_ignore` semantics exactly. That is the price of the compatibility contract, paid once and shared with #1069. +- The bucket's ignore file is recognised by the exact key `.kosli_ignore`. On disk, `ignoreFilePathInTree` also accepted a case-folded spelling such as `.KOSLI_IGNORE` where the operator's filesystem folded case, so on macOS or Windows such a bucket had its rules applied; now it does not, its exclusions stop applying and its fingerprint moves to the Linux value. Same correction as the key case above, and release-noted with it. +- `...`, `.. `, `CON`, colon and backslash keys snapshot again. `..` segments and colliding keys remain errors and now name every key involved. +- Excluded and ignored objects are not downloaded. Each object's temp file is removed once hashed, so peak temp disk falls from the whole bucket to one object, and to the configured budget once #1167 lands. The other half of that trade is memory: the old loop streamed the listing a page at a time, while a tree cannot be fingerprinted without holding it, so peak memory rises from one listing page to every object that survives the filters, at a few hundred bytes per object on top of its key across the listing, the key-to-path maps, the manifest and the tree. A million objects is a few hundred megabytes. #1167 sizes its budget against this figure. +- #1069 rebases onto the shared layer: metadata mode becomes a sha256 source plugged into the same list, normalise, exclude, tree pipeline, its key rule disappears in favour of rule 2, and its rejection of buckets with a root `.kosli_ignore` becomes a download of that one object. +- Delivery is two pull requests. The first carries this decision with sequential downloads, so the security-relevant review is not mixed with performance work. The second, #1167, adds parallel downloads, the byte budget and the flags that tune them. diff --git a/internal/aws/aws.go b/internal/aws/aws.go index 63bddfc7e..8787dffbf 100644 --- a/internal/aws/aws.go +++ b/internal/aws/aws.go @@ -6,13 +6,12 @@ import ( "encoding/hex" "errors" "fmt" - "io/fs" + "io" "os" - "path/filepath" + "path" "regexp" "strings" "sync" - "syscall" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -28,7 +27,6 @@ import ( "github.com/kosli-dev/cli/internal/digest" "github.com/kosli-dev/cli/internal/filters" "github.com/kosli-dev/cli/internal/logger" - "github.com/kosli-dev/cli/internal/utils" ) // EcsEnvRequest represents the PUT request body to be sent to kosli from ECS @@ -421,37 +419,13 @@ func compilePathRegex(patterns []string) ([]*regexp.Regexp, error) { return compiled, nil } -// containsSingleFile checks if a path contains only a single file -func containsSingleFile(directoryPath string) (bool, string, error) { - files, err := os.ReadDir(directoryPath) - if err != nil { - return false, "", err - } - - if len(files) == 1 { - fileInfo := files[0] - - if fileInfo.IsDir() { - // If it's a directory, recursively check inside - subDir := filepath.Join(directoryPath, fileInfo.Name()) - return containsSingleFile(subDir) - } - - // If it's a file, return information about it - path := filepath.Join(directoryPath, fileInfo.Name()) - return true, path, nil - } - - return false, "", nil -} - // objectMatchesFilter reports whether key matches any of the filter entries. // A key matches when it is prefixed by one of paths (literal prefix match) // or when one of patterns matches the full key. func objectMatchesFilter(key string, paths []string, patterns []*regexp.Regexp) bool { - for _, path := range paths { - path = strings.TrimLeft(path, "/") - if strings.HasPrefix(key, path) { + for _, prefix := range paths { + prefix = strings.TrimLeft(prefix, "/") + if strings.HasPrefix(key, prefix) { return true } } @@ -488,126 +462,208 @@ func getS3DataFromClient(client S3API, bucket string, includePaths, includeRegex return s3Data, err } - tempDirName, err := os.MkdirTemp("", "bucketContent") + objects, err := listMatchingS3Objects(client, bucket, includePaths, includeRegexCompiled, excludePaths, excludeRegexCompiled) if err != nil { return s3Data, err } - defer func() { - if err := os.RemoveAll(tempDirName); err != nil { - logger.Warn("failed to remove temp dir %s: %v", tempDirName, err) + if len(objects) == 0 { + return s3Data, fmt.Errorf("no matching file or dirs in bucket: [%s]", bucket) + } + + newest := objects[0].lastModified + for _, object := range objects { + if object.lastModified.After(newest) { + newest = object.lastModified } - }() + } + if newest.IsZero() { + return s3Data, fmt.Errorf("bucket [%s] reported no modification time for any matching object", bucket) + } - params := &s3.ListObjectsV2Input{ - Bucket: aws.String(bucket), + artifactName, sha256, err := fingerprintS3Objects(client, bucket, objects, logger) + if err != nil { + return s3Data, err } - var lastModifiedTime *time.Time - paginator := s3.NewListObjectsV2Paginator(client, params) + s3Data = append(s3Data, &S3Data{Digests: map[string]string{artifactName: sha256}, LastModifiedTimestamp: newest.Unix()}) + + return s3Data, nil +} + +// s3Object is one listed object that survived the include and exclude filters. +type s3Object struct { + key string + lastModified time.Time +} + +// listMatchingS3Objects lists the bucket, dropping folder markers and keys the +// filters exclude, in the order S3 returns them. +func listMatchingS3Objects(client S3ListAPI, bucket string, includePaths []string, includeRegex []*regexp.Regexp, + excludePaths []string, excludeRegex []*regexp.Regexp) ([]s3Object, error) { + objects := []s3Object{} + seen := map[string]bool{} + paginator := s3.NewListObjectsV2Paginator(client, &s3.ListObjectsV2Input{ + Bucket: aws.String(bucket), + }) for paginator.HasMorePages() { - objects, err := paginator.NextPage(context.TODO()) + page, err := paginator.NextPage(context.TODO()) if err != nil { - return s3Data, err + return nil, err } - - for _, object := range objects.Contents { + for _, object := range page.Contents { + // Real S3 always sets both fields; S3-compatible stores may not. + // Dropping an entry with no key would lose an object silently. + if object.Key == nil { + return nil, fmt.Errorf("bucket [%s] listed an object with no key", bucket) + } if strings.HasSuffix(*object.Key, "/") { // skip folders continue } - if shouldExcludePath(*object.Key, includePaths, includeRegexCompiled, excludePaths, excludeRegexCompiled) { + if shouldExcludePath(*object.Key, includePaths, includeRegex, excludePaths, excludeRegex) { continue } - err := downloadFileFromBucket(client, tempDirName, *object.Key, bucket, logger) - if err != nil { - return s3Data, err + // A key listed twice is a listing fault, not a collision between two + // keys, and the collision report relies on keys being distinct. Only + // keys that reach the fingerprint are checked, so memory stays bounded + // by the filtered set rather than the whole bucket. + if seen[*object.Key] { + return nil, fmt.Errorf("bucket [%s] listed object key [%s] more than once", bucket, *object.Key) } - - if lastModifiedTime == nil || object.LastModified.After(*lastModifiedTime) { - lastModifiedTime = object.LastModified + seen[*object.Key] = true + // An object without a timestamp stays in the fingerprint and out of + // the snapshot timestamp, as it was before. + var lastModified time.Time + if object.LastModified != nil { + lastModified = *object.LastModified } + objects = append(objects, s3Object{key: *object.Key, lastModified: lastModified}) } } + return objects, nil +} - if lastModifiedTime == nil { - return s3Data, fmt.Errorf("no matching file or dirs in bucket: [%s]", bucket) +// fingerprintS3Objects fingerprints the objects as the directory their keys +// describe, without ever using a key as a local file name. Each object is +// downloaded to an anonymous temp file, hashed and removed; the fingerprint is +// then computed from the (key, sha256) pairs by digest.VirtualDirSha256, which +// reproduces what digest.DirSha256 gives the same tree on disk. A single object +// is fingerprinted as that file and named after it, as before. +// +// A root .kosli_ignore is downloaded first so its rules can be applied, and +// objects the rules exclude are not downloaded at all. +func fingerprintS3Objects(downloader S3DownloadAPI, bucket string, objects []s3Object, logger *logger.Logger) (string, string, error) { + keys := make([]string, len(objects)) + for i, object := range objects { + keys[i] = object.key + } + paths, err := virtualPathsForS3Keys(keys) + if err != nil { + return "", "", err } - fileSnapshot, artifactPath, err := containsSingleFile(tempDirName) + tempDir, err := os.MkdirTemp("", "bucketContent") if err != nil { - return s3Data, err + return "", "", err } - var sha256 string - artifactName := bucket - if fileSnapshot { - sha256, err = digest.FileSha256(artifactPath, logger) + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + logger.Warn("failed to remove temp dir %s: %v", tempDir, err) + } + }() + + // The manifest starts as paths only; digests are filled in below by index, + // so it stays in listing order. + files := make([]digest.VirtualFile, len(objects)) + for i, object := range objects { + files[i].Path = paths[object.key] + } + + // One object is fingerprinted as that file and named after it, as it was + // when the objects were laid out on disk. + if file, ok := digest.SingleVirtualFile(files); ok { + sha256, err := downloadAndHashS3Object(downloader, tempDir, bucket, objects[0].key, nil, logger) if err != nil { - return s3Data, err + return "", "", err } - artifactName = filepath.Base(artifactPath) - } else { - sha256, err = digest.DirSha256(tempDirName, []string{}, logger) + return file.Name(), sha256, nil + } + + var rules []string + contentSha256 := map[string]string{} + for _, key := range keys { + if paths[key] != digest.IgnoreFileName { + continue + } + sha256, err := downloadAndHashS3Object(downloader, tempDir, bucket, key, func(file *os.File) error { + if _, err := file.Seek(0, io.SeekStart); err != nil { + return err + } + parsed, err := digest.ParseIgnoreRules(file) + if err != nil { + return err + } + rules = parsed + return nil + }, logger) if err != nil { - return s3Data, err + return "", "", err } + contentSha256[key] = sha256 + logger.Debug("object key [%s] is the bucket's %s -- excluding paths: %s", key, digest.IgnoreFileName, rules) } - s3Data = append(s3Data, &S3Data{Digests: map[string]string{artifactName: sha256}, LastModifiedTimestamp: lastModifiedTime.Unix()}) + needed, err := digest.FilesNeedingContent(files, rules) + if err != nil { + return "", "", ignoreRuleError(err) + } - return s3Data, nil -} + for i, object := range objects { + sha256, downloaded := contentSha256[object.key] + switch { + case downloaded: // the ignore-file pass already hashed this object + case needed[files[i].Path]: + sha256, err = downloadAndHashS3Object(downloader, tempDir, bucket, object.key, nil, logger) + if err != nil { + return "", "", err + } + default: + logger.Debug("object key [%s] is excluded by %s and is not downloaded", object.key, digest.IgnoreFileName) + } + files[i].Sha256 = sha256 + } -// localPathForS3Key turns an S3 object key into a path under the download -// directory, or rejects it. The containment rule is shared with every other -// place an external name becomes a local path. -func localPathForS3Key(key string) (string, error) { - rel, err := utils.LocalRelativePath(key) + sha256, err := digest.VirtualDirSha256(files, rules, logger) if err != nil { - return "", unusableS3KeyError(key, err) + return "", "", ignoreRuleError(err) } - return rel, nil + return bucket, sha256, nil } -// The key-caused rejections downloadFileFromBucket adds to those of -// utils.LocalRelativePath. -var ( - errParentPrefixIsObject = errors.New("one of its parent prefixes has already been downloaded as an object") - errPathCollision = errors.New("another object already downloaded to the same local path") -) - -// unusableS3KeyError is only for failures the key itself causes. Advising -// exclusion on a machine fault such as a full disk would drop a legitimate -// object from the snapshot. -func unusableS3KeyError(key string, reason error) error { - return fmt.Errorf("object key [%s] cannot be stored as a local file: %w; exclude it with --exclude-regex, or narrow the include filter if one is set", key, reason) +// ignoreRuleError names the bucket's ignore file when one of its rules cannot +// be applied, and leaves any other failure as it is. +func ignoreRuleError(err error) error { + if errors.Is(err, path.ErrBadPattern) { + return fmt.Errorf("the bucket's %s holds a rule that cannot be applied: %w", digest.IgnoreFileName, err) + } + return err } -func downloadFileFromBucket(downloader S3DownloadAPI, dirName, key, bucket string, logger *logger.Logger) error { - rel, err := localPathForS3Key(key) - if err != nil { - return err - } - dest := filepath.Join(dirName, rel) - err = os.MkdirAll(filepath.Dir(dest), 0770) - if errors.Is(err, syscall.ENOTDIR) { - // Legal in S3, impossible on disk: an object "a" and a key under "a/". - return unusableS3KeyError(key, errParentPrefixIsObject) - } +// downloadAndHashS3Object fetches one object into a fresh temp file, lets +// inspect read it when given, returns the sha256 of its content and removes the +// file. The file's name comes from the OS, so nothing about the key reaches the +// filesystem. +func downloadAndHashS3Object(downloader S3DownloadAPI, tempDir, bucket, key string, inspect func(*os.File) error, logger *logger.Logger) (string, error) { + file, err := os.CreateTemp(tempDir, "object-*") if err != nil { - return fmt.Errorf("object key [%s]: %w", key, err) - } - // O_EXCL fails the snapshot when two keys map to one file rather than - // letting the second overwrite the first. Directories are not covered: - // "A/x" and "a/y" share one on a case-insensitive filesystem. - file, err := os.OpenFile(dest, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) - if errors.Is(err, fs.ErrExist) { - return unusableS3KeyError(key, errPathCollision) - } - if err != nil { - return fmt.Errorf("object key [%s]: %w", key, err) + return "", fmt.Errorf("object key [%s]: %w", key, err) } defer func() { + // Close before remove: Windows will not delete an open file. if err := file.Close(); err != nil { - logger.Warn("failed to close file %s: %v", file.Name(), err) + logger.Warn("failed to close temp file for object key [%s]: %v", key, err) + } + if err := os.Remove(file.Name()); err != nil { + logger.Warn("failed to remove temp file for object key [%s]: %v", key, err) } }() @@ -617,13 +673,22 @@ func downloadFileFromBucket(downloader S3DownloadAPI, dirName, key, bucket strin WriterAt: file, }) if err != nil { - return fmt.Errorf("failed to download object key [%s]: %w", key, err) + return "", fmt.Errorf("failed to download object key [%s]: %w", key, err) } if result.ContentLength != nil { - logger.Debug("downloaded", file.Name(), *result.ContentLength, "bytes") + logger.Debug("downloaded object key [%s]: %d bytes", key, *result.ContentLength) } - return nil + if inspect != nil { + if err := inspect(file); err != nil { + return "", fmt.Errorf("object key [%s]: %w", key, err) + } + } + sha256, err := digest.FileSha256(file.Name(), logger) + if err != nil { + return "", fmt.Errorf("failed to hash object key [%s]: %w", key, err) + } + return sha256, nil } // getFilteredECSClusters fetches a filtered set of ECS clusters recursively (50 at a time) and returns a list of ecs Clusters diff --git a/internal/aws/aws_test.go b/internal/aws/aws_test.go index 7f886a5d6..6c6412a15 100644 --- a/internal/aws/aws_test.go +++ b/internal/aws/aws_test.go @@ -3,11 +3,7 @@ package aws import ( "context" "fmt" - "io/fs" - "os" - "path/filepath" "regexp" - "runtime" "testing" "time" @@ -15,7 +11,6 @@ import ( "github.com/kosli-dev/cli/internal/filters" "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/testHelpers" - "github.com/kosli-dev/cli/internal/utils" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -1302,108 +1297,7 @@ func (suite *AWSTestSuite) TestGetS3DataFromClientRejectsKeysWithDotDotSegments( require.Contains(suite.T(), err.Error(), "uploads/user-a/../../protected/release.bin") } -// Accept rows compare joined paths because the helper returns the key -// uncleaned; filepath.Join is what collapses "a//b" and "./a.txt". -func (suite *AWSTestSuite) TestLocalPathForS3Key() { - for _, t := range []struct { - name string - key string - wantPath string // accept: the path filepath.Join(dir, key) produced before this change - wantErr bool - wantErrMsg string - }{ - {name: "an ordinary nested key", key: "protected/release.bin", wantPath: "protected/release.bin"}, - {name: "a plain filename", key: "a.txt", wantPath: "a.txt"}, - {name: "a short nested key", key: "a/z", wantPath: "a/z"}, - {name: "a dotfile", key: ".kosli_ignore", wantPath: ".kosli_ignore"}, - {name: "a key with spaces", key: "file with spaces.txt", wantPath: "file with spaces.txt"}, - {name: "a key with punctuation", key: "weird!*'().txt", wantPath: "weird!*'().txt"}, - {name: "a unicode key", key: "ünïcödé/файл.txt", wantPath: "ünïcödé/файл.txt"}, - {name: "a dot followed by a space is a literal name", key: ". ", wantPath: ". "}, - {name: "a name that merely starts with two dots", key: "..hidden", wantPath: "..hidden"}, - {name: "a backslash key is a literal filename on this OS", key: `dir\file.txt`, wantPath: `dir\file.txt`}, - {name: "a leading slash is trimmed", key: "/etc/passwd", wantPath: "etc/passwd"}, - {name: "doubled leading slashes are trimmed", key: "//x", wantPath: "x"}, - {name: "a leading dot segment is dropped by Join", key: "./a.txt", wantPath: "a.txt"}, - {name: "a doubled interior slash is collapsed by Join", key: "a//b", wantPath: "a/b"}, - {name: "a dot segment is dropped by Join", key: "a/./b", wantPath: "a/b"}, - // filepath.IsLocal rejects reserved device names and colons on Windows only. - {name: "a reserved Windows name", key: "CON", wantPath: "CON", wantErr: runtime.GOOS == "windows", wantErrMsg: "is not a local path"}, - {name: "a drive-looking segment", key: "C:evil", wantPath: "C:evil", wantErr: runtime.GOOS == "windows", wantErrMsg: "is not a local path"}, - {name: "a colon segment", key: "a:b", wantPath: "a:b", wantErr: runtime.GOOS == "windows", wantErrMsg: "is not a local path"}, - { - name: "a traversing key is rejected", - key: "uploads/user-a/../../protected/release.bin", - wantErr: true, - wantErrMsg: `resolves to ".."`, - }, - { - name: "a backslash-separated traversal is rejected", - key: `uploads/user-a/..\..\..\..\Users\Public\kosli-poc.txt`, - wantErr: true, - wantErrMsg: `resolves to ".."`, - }, - { - name: "a short backslash-separated traversal is rejected", - key: `uploads/user-a/..\x`, - wantErr: true, - wantErrMsg: `resolves to ".."`, - }, - {name: "a bare \"..\" is rejected", key: "..", wantErr: true, wantErrMsg: `resolves to ".."`}, - // Windows drops trailing spaces and dots from a name, so these resolve as "..". - {name: "a \"..\" with a trailing space is rejected", key: "a/.. /x", wantErr: true, wantErrMsg: `resolves to ".."`}, - {name: "three dots are rejected", key: "a/.../b", wantErr: true, wantErrMsg: `resolves to ".."`}, - {name: "a bare \"./.\" is rejected", key: "./.", wantErr: true, wantErrMsg: "names no file"}, - {name: "a trailing \"..\" segment is rejected", key: "a/..", wantErr: true, wantErrMsg: `resolves to ".."`}, - {name: "an empty key is rejected", key: "", wantErr: true, wantErrMsg: "names no file"}, - {name: "a bare slash is rejected", key: "/", wantErr: true, wantErrMsg: "names no file"}, - {name: "doubled slashes with nothing else are rejected", key: "//", wantErr: true, wantErrMsg: "names no file"}, - {name: "a bare dot is rejected", key: ".", wantErr: true, wantErrMsg: "names no file"}, - } { - suite.Run(t.name, func() { - got, err := localPathForS3Key(t.key) - if t.wantErr { - require.Error(suite.T(), err) - require.Contains(suite.T(), err.Error(), t.key) - require.Contains(suite.T(), err.Error(), t.wantErrMsg) - return - } - require.NoError(suite.T(), err) - require.Equal(suite.T(), filepath.Join("base", t.wantPath), filepath.Join("base", got)) - }) - } -} - -func (suite *AWSTestSuite) TestLocalPathForS3KeyKeepsTheRejectionSentinel() { - _, err := localPathForS3Key("uploads/../protected/release.bin") - require.ErrorIs(suite.T(), err, utils.ErrPathTraversal) - require.Contains(suite.T(), err.Error(), "object key [uploads/../protected/release.bin]") -} - -func (suite *AWSTestSuite) TestDownloadFileFromBucketRefusesToOverwrite() { - tempDir := suite.T().TempDir() - preexisting := filepath.Join(tempDir, "README.md") - require.NoError(suite.T(), os.WriteFile(preexisting, []byte("pre-existing content\n"), 0666)) - - client := &FakeS3Client{ - Bucket: fakeS3TestBucketName, - Objects: map[string][]byte{ - "README.md": []byte(fakeReadmeBody), - }, - } - - err := downloadFileFromBucket(client, tempDir, "README.md", fakeS3TestBucketName, logger.NewStandardLogger()) - require.Error(suite.T(), err) - require.Contains(suite.T(), err.Error(), "object key [README.md]") - require.Contains(suite.T(), err.Error(), "--exclude-regex") - - content, readErr := os.ReadFile(preexisting) - require.NoError(suite.T(), readErr) - require.Equal(suite.T(), "pre-existing content\n", string(content), - "the pre-existing file must be left untouched, not truncated") -} - -// Neither key holds a ".." segment, so O_EXCL is what catches this pair. +// Neither key holds a ".." segment; both fold onto one virtual path. func (suite *AWSTestSuite) TestGetS3DataFromClientCollidingKeysAreAnError() { client := &FakeS3Client{ Bucket: fakeS3TestBucketName, @@ -1415,13 +1309,13 @@ func (suite *AWSTestSuite) TestGetS3DataFromClientCollidingKeysAreAnError() { _, err := getS3DataFromClient(client, fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) require.Error(suite.T(), err) - // '/' sorts before 'b', so "a//b" downloads first and "a/b" collides. - require.Contains(suite.T(), err.Error(), "object key [a/b]", "the error must name the key that collided") + require.Contains(suite.T(), err.Error(), "[a//b]", "the error must name both colliding keys") + require.Contains(suite.T(), err.Error(), "[a/b]", "the error must name both colliding keys") require.Contains(suite.T(), err.Error(), "--exclude-regex") } -// An object "a" alongside the prefix "a/" is legal in S3 and impossible on a -// filesystem, so MkdirAll fails with ENOTDIR once "a" lands first. +// An object "a" alongside the prefix "a/" is legal in S3 and impossible in a +// directory tree. func (suite *AWSTestSuite) TestGetS3DataFromClientObjectAndPrefixCollideAreAnError() { client := &FakeS3Client{ Bucket: fakeS3TestBucketName, @@ -1433,42 +1327,11 @@ func (suite *AWSTestSuite) TestGetS3DataFromClientObjectAndPrefixCollideAreAnErr _, err := getS3DataFromClient(client, fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) require.Error(suite.T(), err) - require.Contains(suite.T(), err.Error(), "object key [a/b]") + require.Contains(suite.T(), err.Error(), "object key [a]", "the error must name the object") + require.Contains(suite.T(), err.Error(), "object key [a/b]", "the error must name an object under the prefix") require.Contains(suite.T(), err.Error(), "--exclude-regex") } -func (suite *AWSTestSuite) TestDownloadFileFromBucketNamesTheKeyWithoutAdviceOnFilesystemErrors() { - if runtime.GOOS == "windows" { - suite.T().Skip("chmod on a directory does not block file creation on Windows") - } - if os.Getuid() == 0 { - suite.T().Skip("root ignores directory permissions") - } - tempDir := suite.T().TempDir() - require.NoError(suite.T(), os.Chmod(tempDir, 0500)) - suite.T().Cleanup(func() { _ = os.Chmod(tempDir, 0700) }) - - client := &FakeS3Client{ - Bucket: fakeS3TestBucketName, - Objects: map[string][]byte{ - "README.md": []byte(fakeReadmeBody), - "sub/README.md": []byte(fakeReadmeBody), - }, - } - - // A bare key fails in OpenFile; a nested one has a directory left to - // create, so it fails in MkdirAll. - for _, key := range []string{"README.md", "sub/README.md"} { - suite.Run(key, func() { - err := downloadFileFromBucket(client, tempDir, key, fakeS3TestBucketName, logger.NewStandardLogger()) - require.Error(suite.T(), err) - require.ErrorIs(suite.T(), err, fs.ErrPermission) - require.Contains(suite.T(), err.Error(), fmt.Sprintf("object key [%s]", key)) - require.NotContains(suite.T(), err.Error(), "--exclude-regex") - }) - } -} - // Equal fingerprints mean the odd-shaped keys landed on the same paths as the // plain ones. func (suite *AWSTestSuite) TestGetS3DataFromClientKeepsTodaysLayoutForUnusualKeys() { diff --git a/internal/aws/fake_s3.go b/internal/aws/fake_s3.go index 15c8ee7a8..0fc388170 100644 --- a/internal/aws/fake_s3.go +++ b/internal/aws/fake_s3.go @@ -30,6 +30,9 @@ type FakeS3Client struct { // LastModified maps object key to modification time. Keys without an entry // report fakeS3LastModified. LastModified map[string]time.Time + // NoLastModified lists keys whose listing entry carries no LastModified at + // all, as some S3-compatible stores return. + NoLastModified map[string]bool // PageSize controls how many objects are returned per ListObjectsV2 call. // Defaults to 1000 (matching the AWS default) if zero. PageSize int @@ -114,11 +117,14 @@ func (f *FakeS3Client) ListObjectsV2(_ context.Context, params *s3.ListObjectsV2 contents := make([]s3Types.Object, 0, end-start) for _, key := range keys[start:end] { - contents = append(contents, s3Types.Object{ - Key: aws.String(key), - LastModified: aws.Time(f.lastModified(key)), - Size: aws.Int64(int64(len(f.Objects[key]))), - }) + object := s3Types.Object{ + Key: aws.String(key), + Size: aws.Int64(int64(len(f.Objects[key]))), + } + if !f.NoLastModified[key] { + object.LastModified = aws.Time(f.lastModified(key)) + } + contents = append(contents, object) } out := &s3.ListObjectsV2Output{ diff --git a/internal/aws/s3_fingerprint_test.go b/internal/aws/s3_fingerprint_test.go new file mode 100644 index 000000000..4fa6e8240 --- /dev/null +++ b/internal/aws/s3_fingerprint_test.go @@ -0,0 +1,337 @@ +package aws + +import ( + "context" + "os" + "path/filepath" + "sort" + "sync" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/feature/s3/transfermanager" + "github.com/aws/aws-sdk-go-v2/service/s3" + s3Types "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/kosli-dev/cli/internal/digest" + "github.com/kosli-dev/cli/internal/logger" + "github.com/kosli-dev/cli/internal/utils" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type S3FingerprintTestSuite struct { + suite.Suite +} + +// recordingDownloader wraps an S3API and records every DownloadObject call: the +// key, and the local file the bytes were written to. +type recordingDownloader struct { + S3API + mu sync.Mutex + keys []string + files []string + // onDownload runs inside each DownloadObject call, before delegating. + onDownload func(key string, file *os.File) +} + +func (r *recordingDownloader) DownloadObject(ctx context.Context, params *transfermanager.DownloadObjectInput, optFns ...func(*transfermanager.Options)) (*transfermanager.DownloadObjectOutput, error) { + file, _ := params.WriterAt.(*os.File) + r.mu.Lock() + r.keys = append(r.keys, *params.Key) + if file != nil { + r.files = append(r.files, file.Name()) + } + r.mu.Unlock() + if r.onDownload != nil { + r.onDownload(*params.Key, file) + } + return r.S3API.DownloadObject(ctx, params, optFns...) +} + +func (r *recordingDownloader) downloadedKeys() []string { + r.mu.Lock() + defer r.mu.Unlock() + keys := append([]string{}, r.keys...) + sort.Strings(keys) + return keys +} + +func snapshotFake(t *testing.T, client S3API) (artifactName, fingerprint string) { + t.Helper() + data, err := getS3DataFromClient(client, fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(t, err) + require.Len(t, data, 1) + require.Len(t, data[0].Digests, 1) + for name, sha := range data[0].Digests { + return name, sha + } + return "", "" +} + +// TestPinnedFingerprints holds the fingerprints recorded against main before +// content mode stopped writing objects under their keys. They must never move: +// every existing environment snapshot on the server was computed this way. +func (suite *S3FingerprintTestSuite) TestPinnedFingerprints() { + for _, t := range []struct { + name string + objects map[string][]byte + wantArtifactName string + wantFingerprint string + }{ + { + name: "unusual key shapes fold as filepath.Join folded them", + objects: map[string][]byte{ + "/lead.txt": []byte("u\n"), "a//b": []byte("o\n"), "./c.txt": []byte("t\n"), `d\e.txt`: []byte("n\n"), + }, + wantArtifactName: fakeS3TestBucketName, + wantFingerprint: "27e2d8aa07677b7818b8cf101b45c928aa8fbf7d17a8c8efc84469e24a106ec3", + }, + { + name: "a dot sorts before a slash", + objects: map[string][]byte{ + "a.txt": []byte("1"), "a/z": []byte("2"), "a/b/c": []byte("3"), "b": []byte("4"), + }, + wantArtifactName: fakeS3TestBucketName, + wantFingerprint: "aaddb8f3e299e12316d42fa9ac36d4ed7d38c34e7ec002239269c86a033bb9fd", + }, + { + name: "nested prefixes with folder markers", + objects: map[string][]byte{ + "dir/": nil, "dir/sub/": nil, "dir/sub/x.yml": []byte("x"), "dir/y.txt": []byte("y"), "README.md": []byte("r"), + }, + wantArtifactName: fakeS3TestBucketName, + wantFingerprint: "c26910cdb177dde3c6493d18ba1e916b04715cdfc535e4552b5f9831590e933a", + }, + { + name: "a single object is the file, named by its base name", + objects: map[string][]byte{"only/one/file.bin": []byte("solo")}, + wantArtifactName: "file.bin", + wantFingerprint: "5364f2f2fc4f54e9d47ad29cfb08ef430c8153394bf2a0dff5cbe77a0ffef861", + }, + } { + suite.Run(t.name, func() { + name, sha := snapshotFake(suite.T(), &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: t.objects}) + require.Equal(suite.T(), t.wantArtifactName, name) + require.Equal(suite.T(), t.wantFingerprint, sha) + }) + } +} + +// TestMatchesAttestedDirectory is the property the whole command exists for: +// a bucket holding the files of a directory fingerprints as that directory does +// at attestation time, and a single object as that file does. +func (suite *S3FingerprintTestSuite) TestMatchesAttestedDirectory() { + tree := map[string]string{ + "README.md": "# readme\n", + "dummy/dummy_2/template.yml": "key: value\n", + "dummy/other.txt": "other\n", + "a.txt": "a\n", + "a/z": "z\n", + ".kosli_ignore": "logs\n*.tmp\n", + "logs/noise.log": "noise\n", + "scratch.tmp": "tmp\n", + } + root := suite.T().TempDir() + objects := map[string][]byte{} + for p, content := range tree { + require.NoError(suite.T(), utils.CreateFileWithContent(filepath.Join(root, filepath.FromSlash(p)), content)) + objects[p] = []byte(content) + } + attested, err := digest.DirSha256(root, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + name, sha := snapshotFake(suite.T(), &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: objects}) + require.Equal(suite.T(), fakeS3TestBucketName, name) + require.Equal(suite.T(), attested, sha) + + suite.Run("a single object", func() { + path := filepath.Join(suite.T().TempDir(), "release.bin") + require.NoError(suite.T(), utils.CreateFileWithContent(path, "the release")) + attestedFile, err := digest.FileSha256(path, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + name, sha := snapshotFake(suite.T(), &FakeS3Client{Bucket: fakeS3TestBucketName, + Objects: map[string][]byte{"builds/v1/release.bin": []byte("the release")}}) + require.Equal(suite.T(), "release.bin", name) + require.Equal(suite.T(), attestedFile, sha) + }) +} + +// A root .kosli_ignore in the bucket applies its rules, as DirSha256 applies +// them on disk: a bucket with an excluded directory fingerprints as one that +// never held it. +func (suite *S3FingerprintTestSuite) TestHonoursRootKosliIgnore() { + ignore := []byte("logs\n") + _, withLogs := snapshotFake(suite.T(), &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: map[string][]byte{ + ".kosli_ignore": ignore, "app.js": []byte("app"), "logs/a.log": []byte("a"), "logs/deep/b.log": []byte("b"), + }}) + _, withoutLogs := snapshotFake(suite.T(), &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: map[string][]byte{ + ".kosli_ignore": ignore, "app.js": []byte("app"), + }}) + require.Equal(suite.T(), withoutLogs, withLogs) + + _, noIgnore := snapshotFake(suite.T(), &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: map[string][]byte{ + "app.js": []byte("app"), "logs/a.log": []byte("a"), "logs/deep/b.log": []byte("b"), + }}) + require.NotEqual(suite.T(), noIgnore, withLogs, "the ignore file must have had an effect") +} + +// Exactly the objects that contribute content are downloaded: not folder +// markers, not filter-excluded keys, not keys the root .kosli_ignore excludes. +// Nothing else is skipped, so no object is dropped silently. +func (suite *S3FingerprintTestSuite) TestDownloadsExactlyTheContributingObjects() { + client := &recordingDownloader{S3API: &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: map[string][]byte{ + ".kosli_ignore": []byte("logs\n*.tmp\n"), + "app.js": []byte("app"), + "lib/util.js": []byte("util"), + "lib/": nil, + "logs/a.log": []byte("a"), + "logs/deep/b.log": []byte("b"), + "scratch.tmp": []byte("tmp"), + "filtered/out.txt": []byte("out"), + }}} + data, err := getS3DataFromClient(client, fakeS3TestBucketName, nil, nil, []string{"filtered/"}, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Len(suite.T(), data, 1) + require.Equal(suite.T(), []string{".kosli_ignore", "app.js", "lib/util.js"}, client.downloadedKeys()) +} + +// Objects are downloaded to files whose names owe nothing to the key, each is +// removed once hashed, and the download directory is gone at the end. +func (suite *S3FingerprintTestSuite) TestObjectsNeverLandUnderTheirKeyAndDoNotLinger() { + keys := []string{"alpha.bin", "beta/gamma.bin", "delta/epsilon/zeta.bin"} + objects := map[string][]byte{} + for _, key := range keys { + objects[key] = []byte(key) + } + client := &recordingDownloader{S3API: &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: objects}} + client.onDownload = func(key string, file *os.File) { + require.NotNil(suite.T(), file, "the transfer manager must be handed a real file") + require.NotContains(suite.T(), filepath.Base(file.Name()), filepath.Base(key), + "the local file name must owe nothing to the key") + client.mu.Lock() + earlier := append([]string{}, client.files[:len(client.files)-1]...) + client.mu.Unlock() + for _, previous := range earlier { + _, err := os.Stat(previous) + require.ErrorIs(suite.T(), err, os.ErrNotExist, "an earlier object's file must be gone before the next download") + } + } + + _, err := getS3DataFromClient(client, fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Len(suite.T(), client.files, len(keys)) + for _, file := range client.files { + _, err := os.Stat(file) + require.ErrorIs(suite.T(), err, os.ErrNotExist) + _, err = os.Stat(filepath.Dir(file)) + require.ErrorIs(suite.T(), err, os.ErrNotExist, "the download directory must be removed") + } +} + +// A malformed rule in the bucket's .kosli_ignore fails the snapshot and names +// the file, even when the rule points under a prefix the bucket does not have. +func (suite *S3FingerprintTestSuite) TestAMalformedIgnoreRuleFailsTheSnapshot() { + for _, rule := range []string{"[", "nonexistent/a["} { + suite.Run(rule, func() { + client := &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: map[string][]byte{ + ".kosli_ignore": []byte(rule + "\n"), "app.js": []byte("app"), + }} + _, err := getS3DataFromClient(client, fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "the bucket's .kosli_ignore holds a rule that cannot be applied") + require.Contains(suite.T(), err.Error(), rule) + }) + } +} + +func (suite *S3FingerprintTestSuite) TestADownloadErrorNamesTheKey() { + client := &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: map[string][]byte{ + "README.md": []byte(fakeReadmeBody), "notes.txt": []byte(fakeNotesBody), + }, DownloadObjectErr: os.ErrDeadlineExceeded} + _, err := getS3DataFromClient(client, fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.ErrorIs(suite.T(), err, os.ErrDeadlineExceeded) + require.Contains(suite.T(), err.Error(), "object key [README.md]") + require.NotContains(suite.T(), err.Error(), "--exclude-regex", "a transport failure must not advise dropping the object") +} + +// The temp file's name says nothing about the object, so a failure while +// hashing it must name the key, as every other failure in the path does. +func (suite *S3FingerprintTestSuite) TestAHashErrorNamesTheKey() { + client := &FakeS3Client{Bucket: fakeS3TestBucketName, Objects: map[string][]byte{"README.md": []byte(fakeReadmeBody)}} + // Deleting the file between download and hash is the one way to make the + // hash fail without touching permissions. + _, err := downloadAndHashS3Object(client, suite.T().TempDir(), fakeS3TestBucketName, "README.md", func(file *os.File) error { + return os.Remove(file.Name()) + }, logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.ErrorIs(suite.T(), err, os.ErrNotExist) + require.Contains(suite.T(), err.Error(), "failed to hash object key [README.md]") + require.NotContains(suite.T(), err.Error(), "--exclude-regex") +} + +// Some S3-compatible stores list objects without a LastModified. Such an +// object still belongs in the fingerprint; only the snapshot timestamp is +// computed without it, and a listing with no timestamps at all is an error +// rather than a panic or a zero timestamp. +func (suite *S3FingerprintTestSuite) TestAListingWithoutModificationTimesDoesNotPanic() { + objects := map[string][]byte{"README.md": []byte(fakeReadmeBody), "notes.txt": []byte(fakeNotesBody)} + later := fakeS3LastModified.Add(time.Hour) + full, err := getS3DataFromClient(&FakeS3Client{Bucket: fakeS3TestBucketName, Objects: objects, + LastModified: map[string]time.Time{"notes.txt": later}}, fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + partial, err := getS3DataFromClient(&FakeS3Client{Bucket: fakeS3TestBucketName, Objects: objects, + LastModified: map[string]time.Time{"notes.txt": later}, NoLastModified: map[string]bool{"README.md": true}}, + fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Equal(suite.T(), full[0].Digests, partial[0].Digests, "the object without a timestamp stays in the fingerprint") + require.Equal(suite.T(), later.Unix(), partial[0].LastModifiedTimestamp) + + _, err = getS3DataFromClient(&FakeS3Client{Bucket: fakeS3TestBucketName, Objects: objects, + NoLastModified: map[string]bool{"README.md": true, "notes.txt": true}}, + fakeS3TestBucketName, nil, nil, nil, nil, logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "modification time") +} + +// A listing entry with no key cannot be fingerprinted or reported, and dropping +// it would lose an object silently, so it is an error. +func (suite *S3FingerprintTestSuite) TestAListingEntryWithoutAKeyIsAnError() { + page := &s3.ListObjectsV2Output{Contents: []s3Types.Object{ + {Key: aws.String("README.md"), LastModified: aws.Time(fakeS3LastModified)}, + {LastModified: aws.Time(fakeS3LastModified)}, + }} + _, err := listMatchingS3Objects(singlePageLister{page: page}, fakeS3TestBucketName, nil, nil, nil, nil) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "no key") +} + +// A key listed twice is a listing fault, not two objects: it must not be +// reported as a key colliding with itself, and the advice to exclude it would +// drop the only copy. +func (suite *S3FingerprintTestSuite) TestAListingThatRepeatsAKeyIsAnError() { + page := &s3.ListObjectsV2Output{Contents: []s3Types.Object{ + {Key: aws.String("a"), LastModified: aws.Time(fakeS3LastModified)}, + {Key: aws.String("a"), LastModified: aws.Time(fakeS3LastModified)}, + }} + _, err := listMatchingS3Objects(singlePageLister{page: page}, fakeS3TestBucketName, nil, nil, nil, nil) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "object key [a] more than once") + require.NotContains(suite.T(), err.Error(), "--exclude-regex") +} + +// singlePageLister answers every ListObjectsV2 call with one fixed page. +type singlePageLister struct { + page *s3.ListObjectsV2Output +} + +func (l singlePageLister) ListObjectsV2(context.Context, *s3.ListObjectsV2Input, ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) { + return l.page, nil +} + +func TestS3FingerprintTestSuite(t *testing.T) { + suite.Run(t, new(S3FingerprintTestSuite)) +} diff --git a/internal/aws/s3_keys.go b/internal/aws/s3_keys.go new file mode 100644 index 000000000..084895ce6 --- /dev/null +++ b/internal/aws/s3_keys.go @@ -0,0 +1,148 @@ +package aws + +import ( + "fmt" + "path" + "sort" + "strings" +) + +// maxReportedS3KeyProblems caps how many problems one error lists and how many +// keys one collision names before summarising the rest, so a bucket-wide +// problem stays readable: at most ten lines of at most ten keys each. +const maxReportedS3KeyProblems = 10 + +// virtualPathForS3Key returns the path an object key occupies in the virtual +// tree that is fingerprinted, or rejects a key no directory tree can hold. +// +// Nothing is ever created under the returned path, so the operating system's +// naming rules do not apply: reserved names, colons, backslashes and overlong +// components are all ordinary names here. The fold of "." segments, doubled +// slashes and a leading slash is what filepath.Join did when objects were +// written under their keys, and keeps existing fingerprints unchanged. +// +// A ".." segment is checked on the raw key rather than left to path.Clean, +// which would fold "a/../b" onto "b" silently. DirSha256 walks real +// directories, which never hold an entry named ".", ".." or "", so rejecting +// these keeps every virtual fingerprint inside the space an attested directory +// can match. +func virtualPathForS3Key(key string) (string, error) { + trimmed := strings.TrimLeft(key, "/") + if trimmed == "" { + return "", s3KeyProblem(key, "names no file") + } + if strings.HasSuffix(trimmed, "/") { + return "", s3KeyProblem(key, "is a folder marker, not an object") + } + for _, segment := range strings.Split(trimmed, "/") { + if segment == ".." { + return "", s3KeyProblem(key, `contains a ".." segment`) + } + } + cleaned := path.Clean(trimmed) + if cleaned == "." { + return "", s3KeyProblem(key, "names no file") + } + return cleaned, nil +} + +// virtualPathsForS3Keys maps every object key to its virtual path, or reports +// every key that cannot take part in one directory tree: keys the rule above +// rejects, keys that fold onto the same path, and an object whose path is also +// a directory holding other objects. All problems are reported together so one +// run tells the operator about every key they need to act on. +// +// Folder markers (keys ending in "/") are filtered out by the caller before +// listing reaches here; the rule above rejects any that slip through so they +// can never be mistaken for objects. +func virtualPathsForS3Keys(keys []string) (map[string]string, error) { + paths := make(map[string]string, len(keys)) + keysByPath := map[string][]string{} + problems := []string{} + + for _, key := range keys { + virtualPath, err := virtualPathForS3Key(key) + if err != nil { + problems = append(problems, err.Error()) + continue + } + paths[key] = virtualPath + keysByPath[virtualPath] = append(keysByPath[virtualPath], key) + } + + for virtualPath, colliding := range keysByPath { + if len(colliding) > 1 { + sort.Strings(colliding) + // Folding variants of one path are cheap to write, so one collision + // is bounded the same way the list of problems is. + named, more := colliding, "" + if len(named) > maxReportedS3KeyProblems { + named = named[:maxReportedS3KeyProblems] + more = fmt.Sprintf(" and %d more keys", len(colliding)-maxReportedS3KeyProblems) + } + problems = append(problems, fmt.Sprintf("object keys %s%s fingerprint as the same path [%s]", + bracketed(named), more, virtualPath)) + } + } + + // The lexically smallest key under each directory stands as the example in + // the message, so the report does not depend on listing order. + exampleObjectUnder := map[string]string{} + for virtualPath, keysHere := range keysByPath { + sort.Strings(keysHere) + for dir := path.Dir(virtualPath); dir != "."; dir = path.Dir(dir) { + if existing, ok := exampleObjectUnder[dir]; !ok || keysHere[0] < existing { + exampleObjectUnder[dir] = keysHere[0] + } + } + } + for virtualPath, keysHere := range keysByPath { + child, isAlsoDir := exampleObjectUnder[virtualPath] + if !isAlsoDir { + continue + } + for _, key := range keysHere { + problems = append(problems, fmt.Sprintf("object key [%s] fingerprints as [%s], which is also a directory holding object key [%s]", + key, virtualPath, child)) + } + } + + if len(problems) > 0 { + return nil, s3KeyProblemsError(problems) + } + return paths, nil +} + +// s3KeyProblem describes a failure the key itself causes. The advice belongs +// only on such failures: suggesting exclusion for a machine fault would drop a +// legitimate object from the snapshot. +func s3KeyProblem(key, reason string) error { + return fmt.Errorf("object key [%s] cannot be fingerprinted: %s", key, reason) +} + +// s3KeyProblemsError joins every key problem into one error with the remedy. +func s3KeyProblemsError(problems []string) error { + // Map iteration supplied these in any order; sorting keeps the message stable. + sort.Strings(problems) + if len(problems) == 1 { + return fmt.Errorf("%s; exclude the affected keys with --exclude-regex, or narrow the include filter if one is set", problems[0]) + } + + shown := problems + suffix := "" + if len(shown) > maxReportedS3KeyProblems { + shown = shown[:maxReportedS3KeyProblems] + suffix = fmt.Sprintf("\n(and %d more)", len(problems)-maxReportedS3KeyProblems) + } + return fmt.Errorf("%d problems prevent the bucket from being fingerprinted:\n%s%s\nexclude the keys with --exclude-regex, or narrow the include filter if one is set", + len(problems), strings.Join(shown, "\n"), suffix) +} + +// bracketed formats keys as "[a], [b]" for error messages. +func bracketed(keys []string) string { + parts := make([]string, len(keys)) + for i, key := range keys { + parts[i] = "[" + key + "]" + } + return strings.Join(parts, ", ") +} diff --git a/internal/aws/s3_keys_test.go b/internal/aws/s3_keys_test.go new file mode 100644 index 000000000..d87ece1ea --- /dev/null +++ b/internal/aws/s3_keys_test.go @@ -0,0 +1,251 @@ +package aws + +import ( + "fmt" + "math/rand" + "path" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type S3KeysTestSuite struct { + suite.Suite +} + +// Accepted keys land on exactly the path filepath.Join(dir, key) produced on +// Linux before objects stopped being written under their own names, so every +// bucket that fingerprints today keeps its fingerprint. +func (suite *S3KeysTestSuite) TestVirtualPathForS3Key() { + longComponent := strings.Repeat("n", 300) + for _, t := range []struct { + name string + key string + wantPath string + wantErrMsg string + }{ + {name: "an ordinary nested key", key: "protected/release.bin", wantPath: "protected/release.bin"}, + {name: "a plain filename", key: "a.txt", wantPath: "a.txt"}, + {name: "a short nested key", key: "a/z", wantPath: "a/z"}, + {name: "a dotfile", key: ".kosli_ignore", wantPath: ".kosli_ignore"}, + {name: "a key with spaces", key: "file with spaces.txt", wantPath: "file with spaces.txt"}, + {name: "a key with punctuation", key: "weird!*'().txt", wantPath: "weird!*'().txt"}, + {name: "a unicode key", key: "ünïcödé/файл.txt", wantPath: "ünïcödé/файл.txt"}, + {name: "a dot followed by a space is a literal name", key: ". ", wantPath: ". "}, + {name: "a name that merely starts with two dots", key: "..hidden", wantPath: "..hidden"}, + {name: "a leading slash is trimmed", key: "/etc/passwd", wantPath: "etc/passwd"}, + {name: "doubled leading slashes are trimmed", key: "//x", wantPath: "x"}, + {name: "a leading dot segment is folded", key: "./a.txt", wantPath: "a.txt"}, + {name: "a doubled interior slash is folded", key: "a//b", wantPath: "a/b"}, + {name: "a dot segment is folded", key: "a/./b", wantPath: "a/b"}, + // Nothing is created under these names, so the operating system's rules + // about them no longer apply. + {name: "a backslash is a literal character", key: `dir\file.txt`, wantPath: `dir\file.txt`}, + {name: "a leading backslash is a literal character", key: `\evil.txt`, wantPath: `\evil.txt`}, + {name: "a backslash-separated \"..\" is a literal name", key: `uploads/user-a/..\..\x`, wantPath: `uploads/user-a/..\..\x`}, + {name: "a reserved Windows name", key: "CON", wantPath: "CON"}, + {name: "a drive-looking segment", key: "C:evil", wantPath: "C:evil"}, + {name: "a colon segment", key: "a:b", wantPath: "a:b"}, + {name: "three dots", key: "a/.../b", wantPath: "a/.../b"}, + {name: "two dots and a space", key: "a/.. /x", wantPath: "a/.. /x"}, + {name: "a component longer than any filesystem allows", key: "dir/" + longComponent, wantPath: "dir/" + longComponent}, + {name: "a key of the maximum S3 length", key: strings.Repeat("s/", 511) + "ab", wantPath: strings.Repeat("s/", 511) + "ab"}, + // Rejected: these cannot be entries of any directory tree. + {name: "a traversing key", key: "uploads/user-a/../../protected/release.bin", wantErrMsg: `contains a ".." segment`}, + {name: "a bare \"..\"", key: "..", wantErrMsg: `contains a ".." segment`}, + {name: "a leading \"..\"", key: "../x", wantErrMsg: `contains a ".." segment`}, + {name: "a trailing \"..\"", key: "a/..", wantErrMsg: `contains a ".." segment`}, + {name: "a \"..\" that would fold onto a sibling", key: "a/../b", wantErrMsg: `contains a ".." segment`}, + {name: "an empty key", key: "", wantErrMsg: "names no file"}, + {name: "a bare slash", key: "/", wantErrMsg: "names no file"}, + {name: "doubled slashes with nothing else", key: "//", wantErrMsg: "names no file"}, + {name: "a bare dot", key: ".", wantErrMsg: "names no file"}, + {name: "a dot slash dot", key: "./.", wantErrMsg: "names no file"}, + {name: "a folder marker", key: "dir/", wantErrMsg: "folder marker"}, + {name: "a folder marker with a dot segment", key: "a/./", wantErrMsg: "folder marker"}, + } { + suite.Run(t.name, func() { + got, err := virtualPathForS3Key(t.key) + if t.wantErrMsg != "" { + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), fmt.Sprintf("object key [%s]", t.key)) + require.Contains(suite.T(), err.Error(), t.wantErrMsg) + return + } + require.NoError(suite.T(), err) + require.Equal(suite.T(), t.wantPath, got) + }) + } +} + +// Any key S3 accepts is representable unless it holds a ".." segment, whatever +// operating system runs the snapshot. Components are drawn from an alphabet with +// no '/' and are never exactly "..", so every generated key must be accepted. +func (suite *S3KeysTestSuite) TestEveryS3KeyIsRepresentable() { + alphabet := []rune("abcXYZ019 !-_.*'()&$@=;:+,?\\\t\x01ünï文файл") + random := rand.New(rand.NewSource(20260911)) + component := func() string { + for { + length := 1 + random.Intn(40) + runes := make([]rune, length) + for i := range runes { + runes[i] = alphabet[random.Intn(len(alphabet))] + } + if s := string(runes); s != ".." { + return s + } + } + } + + for i := 0; i < 2000; i++ { + depth := 1 + random.Intn(6) + components := make([]string, depth) + for j := range components { + components[j] = component() + } + if components[0] == "." { + components[0] = "x" + } + key := strings.Join(components, "/") + if len(key) > 1024 { + continue + } + + got, err := virtualPathForS3Key(key) + require.NoError(suite.T(), err, "key %q must be representable", key) + require.Equal(suite.T(), path.Clean(key), got) + require.NotContains(suite.T(), strings.Split(got, "/"), "..") + } +} + +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysMapsEveryKey() { + got, err := virtualPathsForS3Keys([]string{"README.md", "/lead.txt", "a//b", "./c.txt", `d\e.txt`}) + require.NoError(suite.T(), err) + require.Equal(suite.T(), map[string]string{ + "README.md": "README.md", + "/lead.txt": "lead.txt", + "a//b": "a/b", + "./c.txt": "c.txt", + `d\e.txt`: `d\e.txt`, + }, got) +} + +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysNamesEveryCollidingKey() { + _, err := virtualPathsForS3Keys([]string{"x", "a/b", "a//b", "./a/b"}) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "[./a/b]") + require.Contains(suite.T(), err.Error(), "[a//b]") + require.Contains(suite.T(), err.Error(), "[a/b]") + require.Contains(suite.T(), err.Error(), "--exclude-regex") + require.NotContains(suite.T(), err.Error(), "[x]", "an unaffected key must not be named") +} + +// An object "a" beside objects under "a/" is legal in S3 and impossible in a +// directory tree, whichever order S3 lists them in. +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysObjectAndPrefix() { + for _, keys := range [][]string{ + {"a", "a/b"}, + {"a/b", "a"}, + {"a/b/c", "a/b", "z"}, + {"lib", "lib/x", "lib/y"}, + } { + suite.Run(strings.Join(keys, ","), func() { + _, err := virtualPathsForS3Keys(keys) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "also a directory") + require.Contains(suite.T(), err.Error(), "--exclude-regex") + }) + } + + _, err := virtualPathsForS3Keys([]string{"a", "a/b"}) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "object key [a] fingerprints as [a], which is also a directory holding object key [a/b]") +} + +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysReportsEveryBadKey() { + _, err := virtualPathsForS3Keys([]string{"ok.txt", "../one", "two/..", "", "dup", "./dup"}) + require.Error(suite.T(), err) + msg := err.Error() + require.Contains(suite.T(), msg, "4 problems prevent the bucket from being fingerprinted") + require.Contains(suite.T(), msg, "[../one]") + require.Contains(suite.T(), msg, "[two/..]") + require.Contains(suite.T(), msg, "object key []") + require.Contains(suite.T(), msg, "[./dup]") + require.Contains(suite.T(), msg, "--exclude-regex") + require.NotContains(suite.T(), msg, "[ok.txt]") +} + +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysCapsTheReport() { + keys := make([]string, 0, 13) + for i := 0; i < 13; i++ { + keys = append(keys, fmt.Sprintf("%02d/../x", i)) + } + _, err := virtualPathsForS3Keys(keys) + require.Error(suite.T(), err) + msg := err.Error() + require.Contains(suite.T(), msg, "13 problems prevent the bucket from being fingerprinted") + require.Contains(suite.T(), msg, "(and 3 more)") + require.Equal(suite.T(), maxReportedS3KeyProblems, strings.Count(msg, "object key [")) +} + +// Folding variants of one path are cheap to write, so one collision must not +// name every key that folds onto it. +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysCapsTheKeysNamedPerCollision() { + keys := []string{"a/b"} + for i := 0; i < 12; i++ { + keys = append(keys, strings.Repeat("./", i+1)+"a/b") + } + _, err := virtualPathsForS3Keys(keys) + require.Error(suite.T(), err) + msg := err.Error() + require.Contains(suite.T(), msg, "fingerprint as the same path [a/b]") + require.Contains(suite.T(), msg, " and 3 more keys fingerprint as") + require.Equal(suite.T(), maxReportedS3KeyProblems+1, strings.Count(msg, "a/b]"), "the named keys and the path itself") + require.NotContains(suite.T(), msg, "\n", "one problem still reads as one line") +} + +// The reported attack shape from #1155: the traversing key is rejected for its +// ".." segment, and is never allowed to fold onto the object it names. +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysTraversalKeyIsRejected() { + _, err := virtualPathsForS3Keys([]string{ + "protected/release.bin", + "uploads/user-a/../../protected/release.bin", + }) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "object key [uploads/user-a/../../protected/release.bin]") + require.Contains(suite.T(), err.Error(), `contains a ".." segment`) + require.NotContains(suite.T(), err.Error(), "fingerprint as the same path", + "the traversing key must be rejected outright, not reported as a collision") +} + +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysSingleProblemReadsAsOneLine() { + _, err := virtualPathsForS3Keys([]string{"good", "bad/.."}) + require.Error(suite.T(), err) + require.Equal(suite.T(), + `object key [bad/..] cannot be fingerprinted: contains a ".." segment; exclude the affected keys with --exclude-regex, or narrow the include filter if one is set`, + err.Error()) + + // A single problem can still name several keys. + _, err = virtualPathsForS3Keys([]string{"a/b", "a//b"}) + require.Error(suite.T(), err) + require.Equal(suite.T(), + `object keys [a//b], [a/b] fingerprint as the same path [a/b]; exclude the affected keys with --exclude-regex, or narrow the include filter if one is set`, + err.Error()) +} + +func (suite *S3KeysTestSuite) TestVirtualPathsForS3KeysIsDeterministic() { + keys := []string{"c/..", "b/..", "a/..", "d", "d/e"} + first, err := virtualPathsForS3Keys(keys) + require.Error(suite.T(), err) + require.Nil(suite.T(), first) + for i := 0; i < 20; i++ { + _, again := virtualPathsForS3Keys(keys) + require.Equal(suite.T(), err.Error(), again.Error()) + } +} + +func TestS3KeysTestSuite(t *testing.T) { + suite.Run(t, new(S3KeysTestSuite)) +} diff --git a/internal/digest/digest.go b/internal/digest/digest.go index 750330920..4d1e4eed9 100644 --- a/internal/digest/digest.go +++ b/internal/digest/digest.go @@ -32,8 +32,8 @@ var ( "has it been pushed to or pulled from a registry?") ) -// ignoreFileName is the exclusion list a directory artifact may carry at its root. -const ignoreFileName = ".kosli_ignore" +// IgnoreFileName is the exclusion list a directory artifact may carry at its root. +const IgnoreFileName = ".kosli_ignore" // DirSha256 returns sha256 digest of a directory func DirSha256(dirPath string, excludePaths []string, logger *logger.Logger) (string, error) { @@ -235,12 +235,12 @@ func Sha256Fingerprint(parsed godigest.Digest) (string, error) { // stores one spelling and opens any of them. Snapshotting S3 or Azure unzips the // tree onto the machine running the CLI, so that filesystem is the operator's. // -// An exact match wins over a folded one so that ignoreFileName owns the rules +// An exact match wins over a folded one so that IgnoreFileName owns the rules // where a case-sensitive filesystem holds both spellings as distinct files. func ignoreFilePathInTree(dirPath string) (string, error) { // "" is also the answer for a tree with no ignore file, so a swallowed error // would silently mean "no exclusions". - if _, err := os.Lstat(filepath.Join(dirPath, ignoreFileName)); err != nil { + if _, err := os.Lstat(filepath.Join(dirPath, IgnoreFileName)); err != nil { if errors.Is(err, fs.ErrNotExist) { return "", nil } @@ -252,7 +252,7 @@ func ignoreFilePathInTree(dirPath string) (string, error) { } folded := "" for _, entry := range entries { - if !strings.EqualFold(entry.Name(), ignoreFileName) { + if !strings.EqualFold(entry.Name(), IgnoreFileName) { continue } // Only a file can carry rules. The dirent type is not enough on its own: it @@ -265,7 +265,7 @@ func ignoreFilePathInTree(dirPath string) (string, error) { if resolved, err := os.Stat(path); err == nil && resolved.IsDir() { continue } - if entry.Name() == ignoreFileName { + if entry.Name() == IgnoreFileName { return path, nil } if folded == "" { @@ -534,19 +534,8 @@ func excludePathsFromFile(path string) ([]string, error) { fmt.Printf("warning: failed to close file %s: %v\n", path, err) } }() - var excludes = []string{} - scanner := bufio.NewScanner(file) - for scanner.Scan() { - line := scanner.Text() - line = removeComments(line) - line = strings.TrimSpace(line) - if len(line) > 0 { - excludes = append(excludes, line) - } - } - // A stopped scan yields the entries read so far, so an unchecked error means - // fingerprinting against a rule set the file does not hold. - if err := scanner.Err(); err != nil { + excludes, err := ParseIgnoreRules(file) + if err != nil { return nil, fmt.Errorf("failed to read %s: %w", path, err) } return excludes, nil @@ -556,6 +545,29 @@ func excludePathsFromFile(path string) ([]string, error) { return nil, err } +// ParseIgnoreRules reads the content of a .kosli_ignore file: one path or glob +// per line, with "#" starting a comment and blank lines skipped. It is the same +// reading DirSha256 gives the file, for callers that hold the bytes rather than +// a path. +func ParseIgnoreRules(r io.Reader) ([]string, error) { + var rules = []string{} + scanner := bufio.NewScanner(r) + for scanner.Scan() { + line := scanner.Text() + line = removeComments(line) + line = strings.TrimSpace(line) + if len(line) > 0 { + rules = append(rules, line) + } + } + // A stopped scan yields the entries read so far, so an unchecked error means + // fingerprinting against a rule set the file does not hold. + if err := scanner.Err(); err != nil { + return nil, err + } + return rules, nil +} + func removeComments(line string) string { parts := strings.SplitN(line, "#", 2) return strings.TrimRight(parts[0], " ") diff --git a/internal/digest/virtualdir.go b/internal/digest/virtualdir.go new file mode 100644 index 000000000..17f424c6f --- /dev/null +++ b/internal/digest/virtualdir.go @@ -0,0 +1,268 @@ +package digest + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "path" + "sort" + "strings" + + "github.com/kosli-dev/cli/internal/logger" +) + +// VirtualFile is one file in a virtual directory tree: a slash-separated path +// relative to the tree root, plus the hex sha256 of the file's content. +type VirtualFile struct { + // Path is relative to the tree root, slash-separated, with no leading or + // trailing slash and no "." or ".." segments (e.g. "dummy/template.yml"). + // Its depth is bounded by globSeparatorsLimit; S3 keys stay far below it. + Path string + // Sha256 is the hex-encoded sha256 of the file content. + Sha256 string +} + +// Name returns the last segment of the file's path. +func (f VirtualFile) Name() string { + return path.Base(f.Path) +} + +// SingleVirtualFile reports whether the tree holds exactly one file, and +// returns it. +// +// A tree built only from file paths has no empty directories, so a single leaf +// means every level has exactly one child, and two distinct leaves must diverge +// at some node and give it two children. Counting the files is therefore +// equivalent to walking the tree, which is how the same question was answered +// when the objects were laid out on disk, and callers can pick the FileSha256 +// branch on len == 1. +func SingleVirtualFile(files []VirtualFile) (VirtualFile, bool) { + if len(files) != 1 { + return VirtualFile{}, false + } + return files[0], true +} + +// VirtualDirSha256 returns the fingerprint DirSha256 would return for a +// directory containing exactly these files, without touching the disk. +// +// It reproduces calculateDirContentSha256 exactly: walk the tree in +// filepath.WalkDir order -- which is lexical by name within each directory, +// depth-first, with directories and files interleaved -- and append, for every +// entry, the hex sha256 of its base name, plus for files the hex sha256 of +// their content. The fingerprint is the sha256 of that concatenation. +// +// Note that the tree has to be built before sorting: object stores list keys in +// byte order of the whole key, and '.' (0x2E) sorts before '/' (0x2F), so keys +// "a.txt" and "a/z" list as [a.txt, a/z] while WalkDir yields [a, a/z, a.txt]. +// Sorting the flat path list instead of the tree produces a different digest +// whenever a directory shares a name prefix with a sibling file. +// +// ignoreRules are the entries of the tree's root .kosli_ignore, as +// ParseIgnoreRules returns them. They are resolved against the tree exactly as +// DirSha256 resolves them against a directory (see virtualFS), and the root +// .kosli_ignore itself is never excluded by them, so a tree cannot change its +// exclusion list without changing its fingerprint. Callers that hold the file's +// content pass its rules here; the file is an ordinary entry of files. +func VirtualDirSha256(files []VirtualFile, ignoreRules []string, logger *logger.Logger) (string, error) { + if len(files) == 0 { + return "", fmt.Errorf("cannot calculate a fingerprint: no files were provided") + } + + root, err := buildVirtualTree(files) + if err != nil { + return "", err + } + + excluded, err := virtualFS{root: root}.excludedPaths(ignoreRules) + if err != nil { + return "", err + } + + logger.Debug("calculating fingerprint for a virtual tree of %d files -- excluding %d paths", len(files), len(excluded)) + hasher := sha256.New() + err = root.walkIncluded(virtualRoot, excluded, root.protectedVirtualPath(), logger, func(childPath string, child *virtualNode) error { + nameSha256 := sha256OfString(child.name) + hasher.Write([]byte(nameSha256)) //nolint:errcheck // hash.Hash never returns an error + if child.isDir { + logger.Debug("dir: %s -- dirname digest: %s", child.name, nameSha256) + return nil + } + if child.sha256 == "" { + return fmt.Errorf("no content digest for %q, whose content the fingerprint needs", relativeVirtualPath(childPath)) + } + logger.Debug("file: %s -- filename digest: %s -- content digest: %s", child.name, nameSha256, child.sha256) + hasher.Write([]byte(child.sha256)) //nolint:errcheck // hash.Hash never returns an error + return nil + }) + if err != nil { + return "", err + } + return hex.EncodeToString(hasher.Sum(nil)), nil +} + +// FilesNeedingContent reports, by path, which of files VirtualDirSha256 reads +// the content digest of under these ignore rules. Digests on the input are not +// needed and may be empty. A file it leaves out is skipped by the rules, so its +// content need not be fetched and it may later be passed with an empty Sha256 +// without changing the fingerprint. Both run the same walk over the same tree, +// so they cannot disagree. +func FilesNeedingContent(files []VirtualFile, ignoreRules []string) (map[string]bool, error) { + root, err := buildVirtualTree(files) + if err != nil { + return nil, err + } + excluded, err := virtualFS{root: root}.excludedPaths(ignoreRules) + if err != nil { + return nil, err + } + needed := map[string]bool{} + err = root.walkIncluded(virtualRoot, excluded, root.protectedVirtualPath(), nil, func(childPath string, child *virtualNode) error { + if !child.isDir { + needed[relativeVirtualPath(childPath)] = true + } + return nil + }) + if err != nil { + return nil, err + } + return needed, nil +} + +// protectedVirtualPath is the root ignore file, which its own rules never +// exclude. A directory of that name carries no rules, as ignoreFilePathInTree +// decides on disk, so it is not protected either. +func (n *virtualNode) protectedVirtualPath() string { + if child, ok := n.children[IgnoreFileName]; !ok || child.isDir { + return "" + } + return path.Join(virtualRoot, IgnoreFileName) +} + +// relativeVirtualPath strips the synthetic root from a tree path. +func relativeVirtualPath(p string) string { + return strings.TrimPrefix(p, virtualRoot+"/") +} + +// virtualNode is a directory or a file in the virtual tree. Files are leaves +// and carry a content digest; directories carry children keyed by base name. +type virtualNode struct { + name string + sha256 string + isDir bool + children map[string]*virtualNode +} + +// buildVirtualTree turns a flat list of files into a tree, rejecting anything +// that cannot be represented as one: unclean paths, duplicates, and names used +// as both a file and a directory. +func buildVirtualTree(files []VirtualFile) (*virtualNode, error) { + root := &virtualNode{isDir: true, children: map[string]*virtualNode{}} + + for _, file := range files { + if err := validateVirtualPath(file.Path); err != nil { + return nil, err + } + // An empty digest means the content was not read. That is only acceptable + // for a file the rules exclude; VirtualDirSha256 fails on an empty digest + // it reaches, so a skipped download cannot reach a fingerprint. + if file.Sha256 != "" { + if err := ValidateDigest(file.Sha256); err != nil { + return nil, fmt.Errorf("invalid fingerprint for %q: %w", file.Path, err) + } + } + + segments := strings.Split(file.Path, "/") + parent := root + for i, segment := range segments[:len(segments)-1] { + child, ok := parent.children[segment] + if !ok { + child = &virtualNode{name: segment, isDir: true, children: map[string]*virtualNode{}} + parent.children[segment] = child + } + if !child.isDir { + return nil, fmt.Errorf("path %q is both a file and a directory", + strings.Join(segments[:i+1], "/")) + } + parent = child + } + + name := segments[len(segments)-1] + if existing, ok := parent.children[name]; ok { + if existing.isDir { + return nil, fmt.Errorf("path %q is both a file and a directory", file.Path) + } + return nil, fmt.Errorf("duplicate path %q", file.Path) + } + parent.children[name] = &virtualNode{name: name, sha256: file.Sha256} + } + + return root, nil +} + +// walkIncluded visits this node's children in WalkDir order, skipping excluded +// entries as calculateDirContentSha256 does: an excluded directory takes its +// subtree with it, and the protected path is kept whatever the rules say. +// A nil logger is allowed for callers that only want the visits. +func (n *virtualNode) walkIncluded(dir string, excluded map[string]bool, protected string, logger *logger.Logger, + visit func(childPath string, child *virtualNode) error) error { + for _, name := range n.sortedChildNames() { + child := n.children[name] + childPath := path.Join(dir, name) + if excluded[childPath] { + if childPath != protected { + if logger != nil { + logger.Debug("skipping %s as it matches excluded paths", childPath) + } + continue + } + if logger != nil { + logger.Debug("keeping %s although an exclusion matches it: an exclusion list cannot exclude itself", childPath) + } + } + if err := visit(childPath, child); err != nil { + return err + } + if child.isDir { + if err := child.walkIncluded(childPath, excluded, protected, logger, visit); err != nil { + return err + } + } + } + return nil +} + +// sortedChildNames returns child names in the byte order os.ReadDir uses, so +// directories and files interleave exactly as filepath.WalkDir visits them. +func (n *virtualNode) sortedChildNames() []string { + names := make([]string, 0, len(n.children)) + for name := range n.children { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// validateVirtualPath rejects paths that cannot be mapped onto a directory tree +// unambiguously. path.Clean collapses "a//b" to "a/b" and resolves "." and +// "..", so a path that differs from its cleaned form would silently collide +// with, or escape, another entry. +func validateVirtualPath(p string) error { + if p == "" || p != path.Clean(p) || path.IsAbs(p) || strings.HasPrefix(p, "../") || p == ".." { + return fmt.Errorf("path %q is not a clean relative path: it must not be empty, absolute, "+ + "or contain empty, \".\" or \"..\" segments", p) + } + // The walks recurse once per segment over a tree whose shape the bucket's + // writers control, so depth is bounded as filepath.Glob bounds a pattern. + if strings.Count(p, "/") >= globSeparatorsLimit { + return fmt.Errorf("path %q is too deep: at most %d segments are supported", p, globSeparatorsLimit) + } + return nil +} + +// sha256OfString returns the hex sha256 of s. DirSha256 hashes an entry's name +// by writing it to a file and hashing that file, which is the same bytes. +func sha256OfString(s string) string { + sum := sha256.Sum256([]byte(s)) + return hex.EncodeToString(sum[:]) +} diff --git a/internal/digest/virtualdir_test.go b/internal/digest/virtualdir_test.go new file mode 100644 index 000000000..ee2d5a77e --- /dev/null +++ b/internal/digest/virtualdir_test.go @@ -0,0 +1,297 @@ +package digest + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/kosli-dev/cli/internal/logger" + "github.com/kosli-dev/cli/internal/utils" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type VirtualDirTestSuite struct { + suite.Suite + tmpDir string +} + +func (suite *VirtualDirTestSuite) SetupTest() { + suite.tmpDir = suite.T().TempDir() +} + +// TestVirtualDirSha256MatchesDirSha256 is the test that matters: for each tree, +// materialise it on disk, fingerprint it with DirSha256, then fingerprint the +// same (path, content sha256) pairs with VirtualDirSha256 and require the two +// to be identical. Anything VirtualDirSha256 gets wrong about walk order, name +// hashing or nesting shows up here as a mismatch. +func (suite *VirtualDirTestSuite) TestVirtualDirSha256MatchesDirSha256() { + for _, t := range []struct { + name string + files map[string]string // path relative to the tree root -> content + }{ + { + name: "a single file at the root", + files: map[string]string{"README.md": "# readme\n"}, + }, + { + name: "two files at the root", + files: map[string]string{"README.md": "# readme\n", "notes.txt": "notes\n"}, + }, + { + name: "nested directories", + files: map[string]string{ + "README.md": "# readme\n", + "dummy/dummy_2/template.yml": "key: value\n", + "dummy/other.txt": "other\n", + }, + }, + { + // '.' (0x2E) sorts before '/' (0x2F), so a flat sort of the keys + // gives a.txt, a/z -- while WalkDir gives a, a/z, a.txt. Sorting + // the key list instead of the tree fails exactly here. + name: "a dir name sorting between two file names", + files: map[string]string{ + "a.txt": "a\n", + "a/z": "z\n", + "b.txt": "b\n", + }, + }, + { + name: "a deep single-child chain", + files: map[string]string{"a/b/c/d/e/f.txt": "deep\n"}, + }, + { + name: "dot-prefixed and unicode names", + files: map[string]string{ + ".hidden": "hidden\n", + "ünïcode.txt": "unicode\n", + "dir/.keep": "", + "dir/naïve.md": "naive\n", + }, + }, + { + name: "an empty file", + files: map[string]string{"empty.txt": "", "other.txt": "x\n"}, + }, + { + name: "many files across several levels", + files: map[string]string{ + "a.txt": "a\n", "b.txt": "b\n", "c/d.txt": "d\n", "c/e.txt": "e\n", + "c/f/g.txt": "g\n", "c/f/h.txt": "h\n", "i/j.txt": "j\n", + }, + }, + } { + suite.Run(t.name, func() { + root := suite.T().TempDir() + virtualFiles := make([]VirtualFile, 0, len(t.files)) + for path, content := range t.files { + suite.createFile(filepath.Join(root, filepath.FromSlash(path)), content) + virtualFiles = append(virtualFiles, VirtualFile{ + Path: path, + Sha256: sha256OfString(content), + }) + } + + want, err := DirSha256(root, []string{}, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + got, err := VirtualDirSha256(virtualFiles, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + require.Equal(suite.T(), want, got, + "VirtualDirSha256 should equal DirSha256 of the same tree") + }) + } +} + +// TestVirtualDirSha256IgnoresInputOrder pins that the result depends on the tree, +// not on the order S3 happened to list the objects in. +func (suite *VirtualDirTestSuite) TestVirtualDirSha256IgnoresInputOrder() { + files := []VirtualFile{ + {Path: "c/f/g.txt", Sha256: sha256OfString("g")}, + {Path: "a.txt", Sha256: sha256OfString("a")}, + {Path: "c/d.txt", Sha256: sha256OfString("d")}, + {Path: "b.txt", Sha256: sha256OfString("b")}, + } + reversed := make([]VirtualFile, len(files)) + for i, f := range files { + reversed[len(files)-1-i] = f + } + + first, err := VirtualDirSha256(files, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + second, err := VirtualDirSha256(reversed, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + require.Equal(suite.T(), first, second) +} + +func (suite *VirtualDirTestSuite) TestVirtualDirSha256Errors() { + validSha := sha256OfString("x") + for _, t := range []struct { + name string + files []VirtualFile + wantErrMsg string + }{ + { + name: "no files", + files: []VirtualFile{}, + wantErrMsg: "no files", + }, + { + name: "a duplicate path", + files: []VirtualFile{ + {Path: "a.txt", Sha256: validSha}, + {Path: "a.txt", Sha256: validSha}, + }, + wantErrMsg: "duplicate path", + }, + { + name: "a path used as both file and directory", + files: []VirtualFile{ + {Path: "a", Sha256: validSha}, + {Path: "a/b", Sha256: validSha}, + }, + wantErrMsg: "both a file and a directory", + }, + { + name: "a path used as both directory and file", + files: []VirtualFile{ + {Path: "a/b", Sha256: validSha}, + {Path: "a", Sha256: validSha}, + }, + wantErrMsg: "both a file and a directory", + }, + { + name: "an empty path segment", + files: []VirtualFile{{Path: "a//b", Sha256: validSha}}, + wantErrMsg: "not a clean relative path", + }, + { + name: "a parent directory segment", + files: []VirtualFile{{Path: "../evil", Sha256: validSha}}, + wantErrMsg: "not a clean relative path", + }, + { + name: "a current directory segment", + files: []VirtualFile{{Path: "a/./b", Sha256: validSha}}, + wantErrMsg: "not a clean relative path", + }, + { + name: "a leading slash", + files: []VirtualFile{{Path: "/a.txt", Sha256: validSha}}, + wantErrMsg: "not a clean relative path", + }, + { + name: "a trailing slash", + files: []VirtualFile{{Path: "a/", Sha256: validSha}}, + wantErrMsg: "not a clean relative path", + }, + { + name: "an empty path", + files: []VirtualFile{{Path: "", Sha256: validSha}}, + wantErrMsg: "not a clean relative path", + }, + { + // The walks recurse once per segment over a tree the bucket's writers + // shape, so depth is bounded as filepath.Glob bounds a pattern. + name: "a path deeper than the walk bound", + files: []VirtualFile{{Path: strings.Repeat("d/", globSeparatorsLimit) + "x", Sha256: validSha}}, + wantErrMsg: "too deep", + }, + { + name: "an invalid sha256", + files: []VirtualFile{{Path: "a.txt", Sha256: "not-a-digest"}}, + wantErrMsg: "not a valid SHA256 fingerprint", + }, + { + // An empty digest means the content was not read; that is only + // acceptable for a file the rules exclude. + name: "a missing sha256 on a file that is hashed", + files: []VirtualFile{{Path: "a.txt", Sha256: ""}}, + wantErrMsg: "no content digest", + }, + { + name: "an uppercase sha256", + files: []VirtualFile{{Path: "a.txt", Sha256: strings.ToUpper(validSha)}}, + wantErrMsg: "not a valid SHA256 fingerprint", + }, + } { + suite.Run(t.name, func() { + _, err := VirtualDirSha256(t.files, nil, logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), t.wantErrMsg) + }) + } +} + +func (suite *VirtualDirTestSuite) TestSingleVirtualFile() { + validSha := sha256OfString("x") + for _, t := range []struct { + name string + files []VirtualFile + wantOK bool + wantBase string + }{ + { + name: "one file at the root", + files: []VirtualFile{{Path: "README.md", Sha256: validSha}}, + wantOK: true, + wantBase: "README.md", + }, + { + name: "one file nested under prefixes keeps its base name", + files: []VirtualFile{{Path: "dummy/dummy_2/template.yml", Sha256: validSha}}, + wantOK: true, + wantBase: "template.yml", + }, + { + name: "two files is not a single file", + files: []VirtualFile{ + {Path: "a.txt", Sha256: validSha}, + {Path: "b.txt", Sha256: validSha}, + }, + wantOK: false, + }, + { + name: "no files is not a single file", + files: []VirtualFile{}, + wantOK: false, + }, + } { + suite.Run(t.name, func() { + file, ok := SingleVirtualFile(t.files) + require.Equal(suite.T(), t.wantOK, ok) + if t.wantOK { + require.Equal(suite.T(), t.wantBase, file.Name()) + } + }) + } +} + +// TestSingleFileMatchesFileSha256 pins the equivalence the aws package relies on: +// a one-object snapshot is fingerprinted as that file's content digest, exactly +// as content mode did with FileSha256 when the objects were laid out on disk. +func (suite *VirtualDirTestSuite) TestSingleFileMatchesFileSha256() { + content := "the only object\n" + path := filepath.Join(suite.tmpDir, "only.txt") + suite.createFile(path, content) + + want, err := FileSha256(path, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + file, ok := SingleVirtualFile([]VirtualFile{{Path: "nested/only.txt", Sha256: sha256OfString(content)}}) + require.True(suite.T(), ok) + require.Equal(suite.T(), want, file.Sha256) +} + +// createFile writes content to path, creating parent directories as needed. +func (suite *VirtualDirTestSuite) createFile(path, content string) { + suite.T().Helper() + require.NoError(suite.T(), utils.CreateFileWithContent(path, content)) +} + +func TestVirtualDirTestSuite(t *testing.T) { + suite.Run(t, new(VirtualDirTestSuite)) +} diff --git a/internal/digest/virtualglob.go b/internal/digest/virtualglob.go new file mode 100644 index 000000000..75226a441 --- /dev/null +++ b/internal/digest/virtualglob.go @@ -0,0 +1,256 @@ +package digest + +import ( + "fmt" + "path" + "strings" +) + +// virtualRoot stands in for the directory path DirSha256 is given. Every path in +// the virtual tree is spelled "tree/", so a rule joined onto the +// root goes through exactly the string handling filepath.Join, filepath.Glob and +// filepath.Walk apply on disk. +const virtualRoot = "tree" + +// virtualFS answers the questions filepath.Glob and filepath.Walk ask of a +// filesystem, over a virtual tree instead. +// +// DirSha256 resolves .kosli_ignore rules with filepathx.Glob, which has its own +// reading of "**" and inherits filepath.Glob's: a pattern without wildcards is +// returned as written, uncleaned, while one with wildcards is rebuilt from the +// directory listing, cleaned. So "**/x" does find x at the root, but spelled +// "tree//x" (the pieces concatenate around the empty match), and the tree walk +// compares against the cleaned "tree/x", so a root file x survives; a root +// directory x keeps its name while its descendants, joined cleaned by the walk, +// are excluded. "**/*.log" is rebuilt cleaned and matches outright. Reproducing +// the algorithm step for step, rather than its apparent meaning, is what keeps +// the virtual digest equal to the on-disk one for every rule anyone has +// already written. +type virtualFS struct { + root *virtualNode +} + +// excludedPaths resolves ignore rules to the set of tree paths DirSha256 would +// skip, spelled exactly as the resolving glob returned them. +func (fs virtualFS) excludedPaths(rules []string) (map[string]bool, error) { + excluded := map[string]bool{} + for _, rule := range rules { + pattern := path.Join(virtualRoot, rule) + // filepath.Glob validates a pattern before it looks at the filesystem, and + // filepathx hands it the first "**" piece unconditionally, so a malformed + // rule fails on disk even when it names a path outside the tree, which the + // skip below never evaluates. Later pieces are validated only once an + // earlier piece has matched, which for a rule inside the tree happens here + // too. A rule that both leaves the tree and has a malformed later piece + // (say "../**/a[") errors on disk, where the temp directory's parent exists + // and is walked, and is skipped here; modelling that parent would be + // speculative. + if _, err := path.Match(strings.SplitN(pattern, "**", 2)[0], ""); err != nil { + return nil, fmt.Errorf("ignore rule %q: %w", rule, err) + } + // On disk the root is a temp directory with an unguessable name, so a rule + // that resolves to the root or leaves it cannot match anything there, and + // naming the root cannot bring it back, which the cleaned pattern alone + // would not show ("../tree/x" cleans to "tree/x"). + if pattern == virtualRoot || !strings.HasPrefix(pattern, virtualRoot+"/") || escapesVirtualRoot(rule) { + continue + } + matches, err := fs.globDoubleStar(pattern) + if err != nil { + return nil, fmt.Errorf("ignore rule %q: %w", rule, err) + } + for _, match := range matches { + excluded[match] = true + } + } + return excluded, nil +} + +// globDoubleStar mirrors filepathx.Glob: split the pattern on "**", glob each +// piece appended to every match so far, and walk every hit so the next piece is +// tried under all of its descendants. +func (fs virtualFS) globDoubleStar(pattern string) ([]string, error) { + if !strings.Contains(pattern, "**") { + return fs.glob(pattern) + } + matches := []string{""} + for _, piece := range strings.Split(pattern, "**") { + var hits []string + seen := map[string]bool{} + for _, match := range matches { + paths, err := fs.glob(match + piece) + if err != nil { + return nil, err + } + for _, p := range paths { + err := fs.walk(p, func(visited string) { + if !seen[visited] { + hits = append(hits, visited) + seen[visited] = true + } + }) + if err != nil { + return nil, err + } + } + } + matches = hits + } + return matches, nil +} + +// globSeparatorsLimit is filepath's pathSeparatorsLimit: the recursion depth +// at which Glob gives up on a pattern rather than exhaust the stack. It bounds +// the depth of a tree path too, since walking one recurses per segment just +// as globbing recurses per separator. +const globSeparatorsLimit = 10000 + +// glob mirrors filepath.Glob on a Unix filesystem. +func (fs virtualFS) glob(pattern string) ([]string, error) { + return fs.globWithLimit(pattern, 0) +} + +func (fs virtualFS) globWithLimit(pattern string, depth int) ([]string, error) { + // A rule is attacker-writable, so deep wildcard paths take the same bound + // filepath.Glob gives them. + if depth == globSeparatorsLimit { + return nil, path.ErrBadPattern + } + // filepath.Glob rejects a malformed pattern before it looks at the + // filesystem, so a bad rule fails even where nothing could match it. + if _, err := path.Match(pattern, ""); err != nil { + return nil, err + } + if !hasGlobMeta(pattern) { + // A literal pattern is returned as written, not cleaned. + if _, ok := fs.lookup(pattern); !ok { + return nil, nil + } + return []string{pattern}, nil + } + + dir, file := path.Split(pattern) + dir = cleanGlobPath(dir) + if !hasGlobMeta(dir) { + return fs.globDir(dir, file, nil) + } + if dir == pattern { + return nil, path.ErrBadPattern + } + dirs, err := fs.globWithLimit(dir, depth+1) + if err != nil { + return nil, err + } + var matches []string + for _, d := range dirs { + matches, err = fs.globDir(d, file, matches) + if err != nil { + return nil, err + } + } + return matches, nil +} + +// globDir mirrors filepath.glob: match the pattern against each name in one +// directory and return the joined, cleaned paths. +func (fs virtualFS) globDir(dir, pattern string, matches []string) ([]string, error) { + node, ok := fs.lookup(dir) + if !ok || !node.isDir { + return matches, nil + } + for _, name := range node.sortedChildNames() { + matched, err := path.Match(pattern, name) + if err != nil { + return matches, err + } + if matched { + matches = append(matches, path.Join(dir, name)) + } + } + return matches, nil +} + +// walk mirrors filepath.Walk as filepathx drives it: the root is reported as +// given, descendants as joined, cleaned paths, in lexical order. +func (fs virtualFS) walk(root string, visit func(string)) error { + node, ok := fs.lookup(root) + if !ok { + return fmt.Errorf("lstat %s: no such file or directory", root) + } + fs.walkNode(root, node, visit) + return nil +} + +func (fs virtualFS) walkNode(p string, node *virtualNode, visit func(string)) { + visit(p) + if !node.isDir { + return + } + for _, name := range node.sortedChildNames() { + fs.walkNode(path.Join(p, name), node.children[name], visit) + } +} + +// lookup finds the node a possibly uncleaned path spells, or reports that no +// such entry exists. +func (fs virtualFS) lookup(p string) (*virtualNode, bool) { + cleaned := path.Clean(p) + if cleaned == virtualRoot { + return fs.root, true + } + if !strings.HasPrefix(cleaned, virtualRoot+"/") { + return nil, false + } + node := fs.root + for _, segment := range strings.Split(cleaned[len(virtualRoot)+1:], "/") { + if !node.isDir { + return nil, false + } + child, ok := node.children[segment] + if !ok { + return nil, false + } + node = child + } + return node, true +} + +// hasGlobMeta reports whether filepath.Glob would treat the pattern as a glob +// on Unix, where a backslash is an escape character. +func hasGlobMeta(pattern string) bool { + return strings.ContainsAny(pattern, `*?[\`) +} + +// escapesVirtualRoot reports whether the rule walks above the tree root before +// it is cleaned. filepath.Join resolves ".." lexically, so counting segments +// gives the same answer as the join does on disk, where anything above the +// root is outside the tree whatever the rule names next. +func escapesVirtualRoot(rule string) bool { + depth := 0 + for _, segment := range strings.Split(rule, "/") { + switch segment { + case "", ".": + case "..": + if depth == 0 { + return true + } + depth-- + default: + depth++ + } + } + return false +} + +// cleanGlobPath is filepath's helper of the same name: drop the trailing +// separator path.Split leaves on a directory. +func cleanGlobPath(dir string) string { + switch dir { + case "": + return "." + case "/": + return dir + default: + return dir[:len(dir)-1] + } +} diff --git a/internal/digest/virtualignore_test.go b/internal/digest/virtualignore_test.go new file mode 100644 index 000000000..bdb2dbfe6 --- /dev/null +++ b/internal/digest/virtualignore_test.go @@ -0,0 +1,377 @@ +package digest + +import ( + "path" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/kosli-dev/cli/internal/logger" + "github.com/kosli-dev/cli/internal/utils" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type VirtualIgnoreTestSuite struct { + suite.Suite +} + +// ignoreTestTree is shaped to reach every glob behaviour DirSha256 has on disk: +// a directory and a file sharing a stem, the same name at several depths, a +// nested .kosli_ignore, and a directory whose content can be excluded while the +// directory itself stays. +var ignoreTestTree = map[string]string{ + "app.js": "app", + "app.log": "log at the root", + "notes.txt": "notes", + "logs/file1": "c1", + "logs/deep/file2": "c2", + "nested-dir/file1": "n1", + "nested-dir/logs/log.txt": "nl", + "vendor/lib/v.js": "v", + "vendor/lib/.kosli_ignore": "nested-rules", + "a/x": "ax", + "a/b/x": "abx", + "a/b/c/x": "abcx", +} + +// TestMatchesDirSha256 is the test that matters. For each rule set, the tree +// plus a root .kosli_ignore holding the rules is materialised on disk and +// fingerprinted with DirSha256; the same files and rules go through +// VirtualDirSha256 and must give the identical digest. Rows marked hasEffect +// also require the rules to have changed the digest, so a row cannot pass +// because both sides ignored the rules. +func (suite *VirtualIgnoreTestSuite) TestMatchesDirSha256() { + for _, t := range []struct { + name string + ignore string + hasEffect bool + }{ + {name: "no rules", ignore: ""}, + {name: "only comments and blanks", ignore: "# nothing\n\n \n"}, + {name: "a directory by name", ignore: "logs", hasEffect: true}, + {name: "a file by name", ignore: "app.js", hasEffect: true}, + {name: "a directory at depth one", ignore: "*/logs", hasEffect: true}, + {name: "the content of a directory but not the directory", ignore: "logs/*", hasEffect: true}, + {name: "a suffix at the root", ignore: "*.log", hasEffect: true}, + {name: "a suffix at any depth", ignore: "**/*.log", hasEffect: true}, + {name: "a literal name at any depth", ignore: "**/x", hasEffect: true}, + {name: "a literal name at any depth under a prefix", ignore: "a/**/x", hasEffect: true}, + {name: "a directory at any depth", ignore: "**/logs", hasEffect: true}, + {name: "a bare double star", ignore: "**", hasEffect: true}, + // filepathx appends ".log" to every existing path, so this matches only an + // "x.log" whose sibling "x" also exists. Nothing here, on either side. + {name: "a double star glued to a suffix", ignore: "**.log"}, + {name: "a nested ignore file under a prefix", ignore: "vendor/**/.kosli_ignore", hasEffect: true}, + {name: "a trailing slash", ignore: "logs/", hasEffect: true}, + {name: "a leading slash", ignore: "/logs", hasEffect: true}, + {name: "a leading dot segment", ignore: "./logs", hasEffect: true}, + // On disk the root has an unguessable name, so a rule that leaves the + // tree cannot come back by naming it; a rule that only dips through a + // wildcard and returns still folds onto a real path. + {name: "a rule that leaves the tree and names the root", ignore: "../tree/app.js", hasEffect: false}, + {name: "a rule that dips and returns through a wildcard", ignore: "*/../app.js", hasEffect: true}, + {name: "a doubled slash", ignore: "nested-dir//logs", hasEffect: true}, + {name: "a parent segment that leaves the tree", ignore: "../logs"}, + {name: "a rule that matches nothing", ignore: "does-not-exist"}, + // The first piece matches nothing, so the malformed second piece is never + // evaluated, on disk or here. + {name: "a malformed pattern behind a double star that matches nothing", ignore: "nonexistent/**/a["}, + {name: "a star alone", ignore: "*", hasEffect: true}, + {name: "a character class", ignore: "app.[jl]?", hasEffect: true}, + {name: "a question mark", ignore: "a/?", hasEffect: true}, + {name: "an escaped star is a literal", ignore: `app\*`}, + {name: "several rules with a comment", ignore: "logs\n# keep vendor\n*/logs\napp.log # trailing comment\n", hasEffect: true}, + {name: "the ignore file itself", ignore: ".kosli_ignore"}, + {name: "a glob matching the ignore file", ignore: "*ignore*"}, + {name: "a dotted glob matching the ignore file", ignore: ".kosli*"}, + {name: "a double star matching the ignore file", ignore: "**/.kosli_ignore", hasEffect: true}, + } { + suite.Run(t.name, func() { + root := suite.T().TempDir() + files := suite.materialise(root, ignoreTestTree, t.ignore) + + want, err := DirSha256(root, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + rules, err := ParseIgnoreRules(strings.NewReader(t.ignore)) + require.NoError(suite.T(), err) + got, err := VirtualDirSha256(files, rules, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Equal(suite.T(), want, got, "VirtualDirSha256 must equal DirSha256 with rules %q", t.ignore) + + noRules, err := VirtualDirSha256(files, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + if t.hasEffect { + require.NotEqual(suite.T(), noRules, got, "rules %q were expected to change the digest", t.ignore) + } else { + require.Equal(suite.T(), noRules, got, "rules %q were expected to leave the digest unchanged", t.ignore) + } + }) + } +} + +// A malformed pattern fails DirSha256, so it must fail the virtual digest too +// rather than silently excluding nothing. filepath.Glob validates the pattern +// before it looks at the filesystem, so this holds even for a rule under a +// directory the tree does not have. +func (suite *VirtualIgnoreTestSuite) TestMalformedRuleIsAnErrorOnBothSides() { + // The last rule is a wildcard path deeper than filepath.Glob's recursion + // limit, which it rejects rather than descend. + for _, rule := range []string{"[", "nonexistent/a[", "logs/[", "a/**/[", "../a[", "../tree/a[", strings.Repeat("*/", globSeparatorsLimit+1) + "x"} { + suite.Run(rule, func() { + root := suite.T().TempDir() + files := suite.materialise(root, ignoreTestTree, rule) + + _, err := DirSha256(root, nil, logger.NewStandardLogger()) + require.Error(suite.T(), err, "DirSha256 must reject the rule") + + _, err = VirtualDirSha256(files, []string{rule}, logger.NewStandardLogger()) + require.Error(suite.T(), err, "VirtualDirSha256 must reject the rule") + require.ErrorIs(suite.T(), err, path.ErrBadPattern) + require.Contains(suite.T(), err.Error(), rule) + }) + } +} + +// On disk only a file named .kosli_ignore carries rules and is protected from +// them; a directory of that name is an ordinary entry a rule can exclude. The +// rules come from the caller here, since a tree in this shape has no ignore +// file to hold them. +func (suite *VirtualIgnoreTestSuite) TestADirectoryNamedLikeTheIgnoreFileIsNotProtected() { + tree := map[string]string{IgnoreFileName + "/x": "not rules\n", "app.js": "app\n"} + root := suite.T().TempDir() + files := suite.materialise(root, tree, "") + rules := []string{IgnoreFileName} + + want, err := DirSha256(root, rules, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + got, err := VirtualDirSha256(files, rules, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Equal(suite.T(), want, got) + + noRules, err := VirtualDirSha256(files, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.NotEqual(suite.T(), noRules, got, "the rule must exclude the directory") + + needed, err := FilesNeedingContent(files, rules) + require.NoError(suite.T(), err) + require.Equal(suite.T(), map[string]bool{"app.js": true}, needed) +} + +// Mirrors TestDirSha256IgnoreFileCannotHideItself: an ignore file that lists +// itself cannot hide an added file, because the file's own content stays in the +// digest. +func (suite *VirtualIgnoreTestSuite) TestIgnoreFileCannotHideItself() { + baseline := map[string]string{ + "app/index.js": "console.log(1)", + "app/lib/util.js": "exports.x = 1", + } + approvedFiles := virtualFilesFor(baseline, "") + approved, err := VirtualDirSha256(approvedFiles, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + for _, ignore := range []string{ + ".kosli_ignore\napp/backdoor.js", + "*ignore*\napp/backdoor.js", + ".kosli*\napp/backdoor.js", + "**/.kosli_ignore\napp/backdoor.js", + "**\napp/backdoor.js", + } { + suite.Run(ignore, func() { + deployed := map[string]string{} + for k, v := range baseline { + deployed[k] = v + } + deployed["app/backdoor.js"] = "BACKDOOR" + rules, err := ParseIgnoreRules(strings.NewReader(ignore)) + require.NoError(suite.T(), err) + + got, err := VirtualDirSha256(virtualFilesFor(deployed, ignore), rules, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.NotEqual(suite.T(), approved, got, "the added file is invisible to the fingerprint") + }) + } +} + +// Only the root ignore file is protected; a nested one is an ordinary file the +// root list may exclude. +func (suite *VirtualIgnoreTestSuite) TestNestedIgnoreFileIsExcludable() { + rules := "vendor/**/.kosli_ignore" + withNested := virtualFilesFor(map[string]string{ + "app.js": "app", + "vendor/lib/v.js": "v", + "vendor/lib/.kosli_ignore": "nested-rules", + }, rules) + withoutNested := virtualFilesFor(map[string]string{ + "app.js": "app", + "vendor/lib/v.js": "v", + }, rules) + + parsed, err := ParseIgnoreRules(strings.NewReader(rules)) + require.NoError(suite.T(), err) + a, err := VirtualDirSha256(withNested, parsed, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + b, err := VirtualDirSha256(withoutNested, parsed, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Equal(suite.T(), b, a) +} + +// FilesNeedingContent is what lets a caller skip fetching excluded content: it +// must name exactly the files whose digest VirtualDirSha256 reads, so a file it +// leaves out may carry no digest at all without changing the fingerprint. +func (suite *VirtualIgnoreTestSuite) TestFilesNeedingContentAgreesWithTheDigest() { + for _, t := range []struct { + name string + rules []string + want []string + }{ + {name: "no rules hash everything", rules: nil, want: allPaths(ignoreTestTree, true)}, + {name: "a directory takes its files with it", rules: []string{"logs"}, want: without(allPaths(ignoreTestTree, true), "logs/file1", "logs/deep/file2")}, + // "*" matches the subdirectory "deep" as well, and an excluded directory + // takes its subtree with it. + {name: "the content of a directory", rules: []string{"logs/*"}, want: without(allPaths(ignoreTestTree, true), "logs/file1", "logs/deep/file2")}, + {name: "a suffix at any depth", rules: []string{"**/*.log"}, want: without(allPaths(ignoreTestTree, true), "app.log")}, + {name: "the ignore file cannot exclude itself", rules: []string{".kosli_ignore", "**"}, want: []string{".kosli_ignore"}}, + } { + suite.Run(t.name, func() { + needed, err := FilesNeedingContent(virtualFilesFromPaths(allPaths(ignoreTestTree, true)), t.rules) + require.NoError(suite.T(), err) + got := make([]string, 0, len(needed)) + for p := range needed { + got = append(got, p) + } + sort.Strings(got) + sort.Strings(t.want) + require.Equal(suite.T(), t.want, got) + + // Files not needed may carry no digest and the fingerprint is unchanged. + full := virtualFilesFor(ignoreTestTree, "rules") + sparse := make([]VirtualFile, 0, len(full)) + for _, f := range full { + if !needed[f.Path] { + f.Sha256 = "" + } + sparse = append(sparse, f) + } + wantSha, err := VirtualDirSha256(full, t.rules, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + gotSha, err := VirtualDirSha256(sparse, t.rules, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Equal(suite.T(), wantSha, gotSha) + }) + } +} + +func (suite *VirtualIgnoreTestSuite) TestFilesNeedingContentRejectsAMalformedRule() { + _, err := FilesNeedingContent([]VirtualFile{{Path: "a.txt"}}, []string{"["}) + require.Error(suite.T(), err) +} + +func virtualFilesFromPaths(paths []string) []VirtualFile { + files := make([]VirtualFile, len(paths)) + for i, p := range paths { + files[i] = VirtualFile{Path: p} + } + return files +} + +func allPaths(tree map[string]string, withIgnoreFile bool) []string { + paths := make([]string, 0, len(tree)+1) + for p := range tree { + paths = append(paths, p) + } + if withIgnoreFile { + paths = append(paths, IgnoreFileName) + } + sort.Strings(paths) + return paths +} + +func without(paths []string, drop ...string) []string { + kept := make([]string, 0, len(paths)) + for _, p := range paths { + skip := false + for _, d := range drop { + if p == d { + skip = true + } + } + if !skip { + kept = append(kept, p) + } + } + return kept +} + +func (suite *VirtualIgnoreTestSuite) TestParseIgnoreRules() { + for _, t := range []struct { + name string + input string + want []string + }{ + {name: "empty input", input: "", want: []string{}}, + {name: "one rule", input: "logs", want: []string{"logs"}}, + {name: "blank lines and whitespace are dropped", input: "\n logs \n\n\t*.log\t\n", want: []string{"logs", "*.log"}}, + {name: "comment lines are dropped", input: "# all logs\nlogs\n# done", want: []string{"logs"}}, + {name: "a trailing comment is stripped", input: "logs # noisy", want: []string{"logs"}}, + {name: "a comment with no rule before it is dropped", input: " # only a comment", want: []string{}}, + {name: "windows line endings", input: "logs\r\n*.log\r\n", want: []string{"logs", "*.log"}}, + } { + suite.Run(t.name, func() { + got, err := ParseIgnoreRules(strings.NewReader(t.input)) + require.NoError(suite.T(), err) + require.Equal(suite.T(), t.want, got) + }) + } +} + +// ParseIgnoreRules and excludePathsFromFile must agree, since DirSha256 reads +// the file while the virtual digest is handed the same bytes. +func (suite *VirtualIgnoreTestSuite) TestParseIgnoreRulesAgreesWithTheFileReader() { + content := "logs\n# comment\n*/logs # trailing\n\n app.log\n" + path := filepath.Join(suite.T().TempDir(), ".kosli_ignore") + require.NoError(suite.T(), utils.CreateFileWithContent(path, content)) + + fromFile, err := excludePathsFromFile(path) + require.NoError(suite.T(), err) + fromReader, err := ParseIgnoreRules(strings.NewReader(content)) + require.NoError(suite.T(), err) + require.Equal(suite.T(), fromFile, fromReader) +} + +// materialise writes the tree and, when ignore is non-empty, a root +// .kosli_ignore holding it; it returns the matching virtual files. +func (suite *VirtualIgnoreTestSuite) materialise(root string, tree map[string]string, ignore string) []VirtualFile { + suite.T().Helper() + for p, content := range tree { + require.NoError(suite.T(), utils.CreateFileWithContent(filepath.Join(root, filepath.FromSlash(p)), content)) + } + if ignore != "" { + require.NoError(suite.T(), utils.CreateFileWithContent(filepath.Join(root, IgnoreFileName), ignore)) + } + return virtualFilesFor(tree, ignore) +} + +// virtualFilesFor returns the (path, sha256) pairs for tree plus, when ignore +// is non-empty, a root .kosli_ignore with that content, in a fixed order. +func virtualFilesFor(tree map[string]string, ignore string) []VirtualFile { + paths := make([]string, 0, len(tree)+1) + for p := range tree { + paths = append(paths, p) + } + sort.Strings(paths) + files := make([]VirtualFile, 0, len(paths)+1) + for _, p := range paths { + files = append(files, VirtualFile{Path: p, Sha256: sha256OfString(tree[p])}) + } + if ignore != "" { + files = append(files, VirtualFile{Path: IgnoreFileName, Sha256: sha256OfString(ignore)}) + } + return files +} + +func TestVirtualIgnoreTestSuite(t *testing.T) { + suite.Run(t, new(VirtualIgnoreTestSuite)) +}