From 830778911baa99a5c2094ed42e43a1b84a2f1980 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Sat, 1 Aug 2026 23:34:49 +0100 Subject: [PATCH 1/7] feat(digest): add VirtualDirSha256 for in-memory tree fingerprints VirtualDirSha256 returns the fingerprint DirSha256 would produce for a directory containing a given set of files, from (path, content sha256) pairs alone, without touching the disk. SingleVirtualFile reports the one-file case that callers fingerprint with the file's own digest. This lets a caller that can obtain per-file content digests some other way -- an object store that already stores them, say -- reproduce an artifact fingerprint without downloading and hashing every file. The tree is built before sorting, deliberately. 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 filepath.WalkDir yields [a, a/z, a.txt]. Sorting the flat path list would diverge from DirSha256 whenever a directory shares a name prefix with a sibling file. Paths that cannot be mapped onto a tree unambiguously are rejected rather than silently collapsed: unclean paths ("a//b", "..", absolute), duplicates, and names used as both a file and a directory. Every fingerprint test materialises the tree on disk, fingerprints it with DirSha256, and requires VirtualDirSha256 of the same pairs to match, so the two implementations cannot drift. --- internal/digest/virtualdir.go | 176 ++++++++++++++++++ internal/digest/virtualdir_test.go | 283 +++++++++++++++++++++++++++++ 2 files changed, 459 insertions(+) create mode 100644 internal/digest/virtualdir.go create mode 100644 internal/digest/virtualdir_test.go diff --git a/internal/digest/virtualdir.go b/internal/digest/virtualdir.go new file mode 100644 index 000000000..c30f39121 --- /dev/null +++ b/internal/digest/virtualdir.go @@ -0,0 +1,176 @@ +package digest + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "hash" + "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"). + 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. +// +// This mirrors what containsSingleFile decides for a tree on disk: 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, 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. +// +// .kosli_ignore is deliberately not handled here: reading it needs the file's +// content, which the caller may not have, so exclusions stay the caller's +// concern. +func VirtualDirSha256(files []VirtualFile, 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 + } + + logger.Debug("calculating fingerprint for a virtual tree of %d files", len(files)) + hasher := sha256.New() + root.writeDigests(hasher, logger) + return hex.EncodeToString(hasher.Sum(nil)), nil +} + +// 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 + } + 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 +} + +// writeDigests appends this node's children to the hash in WalkDir order. +func (n *virtualNode) writeDigests(hasher hash.Hash, logger *logger.Logger) { + for _, name := range n.sortedChildNames() { + child := n.children[name] + 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) + child.writeDigests(hasher, logger) + continue + } + 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 + } +} + +// 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) + } + 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..93ffd14c8 --- /dev/null +++ b/internal/digest/virtualdir_test.go @@ -0,0 +1,283 @@ +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, 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, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + second, err := VirtualDirSha256(reversed, 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", + }, + { + name: "an invalid sha256", + files: []VirtualFile{{Path: "a.txt", Sha256: "not-a-digest"}}, + wantErrMsg: "not a valid SHA256 fingerprint", + }, + { + 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, 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 does via containsSingleFile + FileSha256. +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)) +} From a31b2d1a3564ac5800dbc0a0413996df2c901ba7 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Sat, 1 Aug 2026 23:34:54 +0100 Subject: [PATCH 2/7] feat(aws): add HeadObject to the S3 seam Fingerprinting a bucket from the checksums S3 already stores needs object metadata, not object content. Add S3HeadAPI to the S3API composite, backed by the same *s3.Client that already serves listing, and give FakeS3Client a HeadObject to match. S3MetadataAPI pairs listing with metadata and deliberately leaves out S3DownloadAPI, so a function typed against it cannot read object content even by accident. The fake models stored checksums sparsely, via a Checksums map rather than deriving them from object bytes: an object uploaded without an explicit checksum algorithm has none, and that is the common case a caller has to handle. It also withholds the checksum unless the request sets ChecksumMode, exactly as S3 does -- a fake that always returned it would hide a caller that forgets to ask. The contract tests gain a sha256ChecksumKey parameter and cover metadata retrieval, the missing-key error, and both sides of the ChecksumMode behaviour. They skip when no checksum-bearing object is available, which is the case for kosli-cli-public today: adding one there would change the golden fingerprints TestGetS3Data pins. --- internal/aws/aws.go | 30 +++++++++-- internal/aws/fake_s3.go | 54 +++++++++++++++++++ internal/aws/s3_contract_test.go | 89 ++++++++++++++++++++++++++++++-- 3 files changed, 167 insertions(+), 6 deletions(-) diff --git a/internal/aws/aws.go b/internal/aws/aws.go index 8766fceb4..4470c0691 100644 --- a/internal/aws/aws.go +++ b/internal/aws/aws.go @@ -149,17 +149,37 @@ type S3DownloadAPI interface { DownloadObject(ctx context.Context, params *transfermanager.DownloadObjectInput, optFns ...func(*transfermanager.Options)) (*transfermanager.DownloadObjectOutput, error) } +// S3HeadAPI reads an object's metadata without reading the object itself, +// including the checksum S3 stores for it. The real *s3.Client satisfies this +// implicitly. +// +// The stored checksum is only returned when the request sets ChecksumMode to +// ChecksumModeEnabled. +type S3HeadAPI interface { + HeadObject(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error) +} + // S3API is the combined S3 surface that GetS3Data depends on. type S3API interface { S3ListAPI S3DownloadAPI + S3HeadAPI +} + +// S3MetadataAPI is the narrower surface needed to fingerprint a bucket from the +// checksums S3 already stores. It deliberately excludes S3DownloadAPI, so the +// type signature alone shows that no object content is read. +type S3MetadataAPI interface { + S3ListAPI + S3HeadAPI } -// s3Client combines the two real AWS clients that back S3API: *s3.Client for -// listing and *transfermanager.Client for downloading. +// s3Client combines the real AWS clients that back S3API: *s3.Client for +// listing and metadata, and *transfermanager.Client for downloading. type s3Client struct { S3ListAPI S3DownloadAPI + S3HeadAPI } // defaultNewS3Client creates a real S3 client from credentials. @@ -168,7 +188,11 @@ func defaultNewS3Client(creds *AWSStaticCreds) (S3API, error) { if err != nil { return nil, err } - return &s3Client{S3ListAPI: client, S3DownloadAPI: transfermanager.New(client)}, nil + return &s3Client{ + S3ListAPI: client, + S3DownloadAPI: transfermanager.New(client), + S3HeadAPI: client, + }, nil } // NewS3ClientFunc is the factory used by GetS3Data to create an S3API client. diff --git a/internal/aws/fake_s3.go b/internal/aws/fake_s3.go index 15c8ee7a8..5480955da 100644 --- a/internal/aws/fake_s3.go +++ b/internal/aws/fake_s3.go @@ -17,6 +17,19 @@ import ( // entry in FakeS3Client.LastModified. var fakeS3LastModified = time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC) +// FakeS3Checksum is the additional checksum S3 has stored for an object. +// Objects uploaded without an explicit checksum algorithm have none, which is +// why FakeS3Client.Checksums is keyed sparsely rather than derived from content. +type FakeS3Checksum struct { + // SHA256 is Base64-encoded, as S3 returns it. A composite (multipart) + // checksum carries a "-N" part-count suffix and is a hash of the part + // hashes, not of the object content. + SHA256 string + // Type is COMPOSITE for multipart uploads and FULL_OBJECT for whole-object + // checksums. + Type s3Types.ChecksumType +} + // FakeS3Client is an in-memory implementation of S3API for testing. // It simulates continuation-token pagination and returns errors for unknown // buckets and missing objects. @@ -30,6 +43,10 @@ type FakeS3Client struct { // LastModified maps object key to modification time. Keys without an entry // report fakeS3LastModified. LastModified map[string]time.Time + // Checksums maps object key to the additional checksum S3 has stored for + // it. A key with no entry has no additional checksum, as objects uploaded + // without --checksum-algorithm do, and HeadObject returns none for it. + Checksums map[string]FakeS3Checksum // PageSize controls how many objects are returned per ListObjectsV2 call. // Defaults to 1000 (matching the AWS default) if zero. PageSize int @@ -39,6 +56,9 @@ type FakeS3Client struct { // DownloadObjectErr, if set, is returned by DownloadObject for any object. // Useful for testing error propagation. DownloadObjectErr error + // HeadObjectErr, if set, is returned by HeadObject for any object. + // Useful for testing error propagation. + HeadObjectErr error } func (f *FakeS3Client) pageSize() int { @@ -135,6 +155,40 @@ func (f *FakeS3Client) ListObjectsV2(_ context.Context, params *s3.ListObjectsV2 return out, nil } +func (f *FakeS3Client) HeadObject(_ context.Context, params *s3.HeadObjectInput, _ ...func(*s3.Options)) (*s3.HeadObjectOutput, error) { + if params.Bucket == nil || params.Key == nil { + return nil, fmt.Errorf("missing required fields: Bucket and Key") + } + if *params.Bucket != f.Bucket { + // Real S3 returns *types.NoSuchBucket. + return nil, fmt.Errorf("bucket not found: %s", *params.Bucket) + } + if f.HeadObjectErr != nil { + return nil, f.HeadObjectErr + } + content, ok := f.Objects[*params.Key] + if !ok { + // Real S3 returns *types.NotFound for HeadObject. + return nil, fmt.Errorf("object not found: %s", *params.Key) + } + + out := &s3.HeadObjectOutput{ + ContentLength: aws.Int64(int64(len(content))), + LastModified: aws.Time(f.lastModified(*params.Key)), + } + + // S3 only returns a stored checksum when the request asks for it. Returning + // it unconditionally would hide a caller that forgets to set ChecksumMode. + if params.ChecksumMode != s3Types.ChecksumModeEnabled { + return out, nil + } + if checksum, ok := f.Checksums[*params.Key]; ok { + out.ChecksumSHA256 = aws.String(checksum.SHA256) + out.ChecksumType = checksum.Type + } + return out, nil +} + func (f *FakeS3Client) DownloadObject(_ context.Context, params *transfermanager.DownloadObjectInput, _ ...func(*transfermanager.Options)) (*transfermanager.DownloadObjectOutput, error) { if params.Bucket == nil || params.Key == nil { return nil, fmt.Errorf("missing required fields: Bucket and Key") diff --git a/internal/aws/s3_contract_test.go b/internal/aws/s3_contract_test.go index ec9ea96d3..20a86e2c1 100644 --- a/internal/aws/s3_contract_test.go +++ b/internal/aws/s3_contract_test.go @@ -2,6 +2,8 @@ package aws import ( "context" + "crypto/sha256" + "encoding/base64" "errors" "os" "path/filepath" @@ -10,6 +12,7 @@ import ( "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/testHelpers" "github.com/stretchr/testify/require" ) @@ -17,6 +20,13 @@ import ( // errInjected is the error tests inject into FakeS3Client to exercise error paths. var errInjected = errors.New("injected error") +// base64Sha256 returns the Base64-encoded SHA256 of content, the form S3 +// reports a stored full-object checksum in. +func base64Sha256(content []byte) string { + sum := sha256.Sum256(content) + return base64.StdEncoding.EncodeToString(sum[:]) +} + // runS3ContractTests exercises the S3API contract. It verifies the behaviours // we depend on — object listing, continuation-token pagination, object // download, and error responses for missing buckets and keys. @@ -26,7 +36,10 @@ var errInjected = errors.New("injected error") // // bucket must name a bucket the client can see, holding at least two objects. // existingKey must name an object in that bucket with a non-empty body. -func runS3ContractTests(t *testing.T, client S3API, bucket, existingKey string) { +// sha256ChecksumKey must name an object stored with an SHA256 checksum, or be +// empty to skip the checksum sub-tests -- kosli-cli-public holds no such object +// yet, and adding one would change the golden fingerprints TestGetS3Data pins. +func runS3ContractTests(t *testing.T, client S3API, bucket, existingKey, sha256ChecksumKey string) { t.Helper() t.Run("ListObjectsV2 returns objects with keys and modification times", func(t *testing.T) { @@ -113,6 +126,60 @@ func runS3ContractTests(t *testing.T, client S3API, bucket, existingKey string) }) require.Error(t, err) }) + + t.Run("HeadObject returns object metadata", func(t *testing.T) { + out, err := client.HeadObject(context.TODO(), &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(existingKey), + }) + require.NoError(t, err) + require.NotNil(t, out) + require.NotNil(t, out.ContentLength, "ContentLength should be present") + require.NotNil(t, out.LastModified, "LastModified should be present") + }) + + t.Run("HeadObject errors for a missing key", func(t *testing.T) { + _, err := client.HeadObject(context.TODO(), &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String("nonexistent-key-that-should-not-exist"), + }) + require.Error(t, err) + }) + + t.Run("HeadObject omits the checksum unless ChecksumMode is enabled", func(t *testing.T) { + if sha256ChecksumKey == "" { + t.Skip("no object with an SHA256 checksum available in this bucket") + } + // S3 only returns stored checksums when asked. A fake that always + // returned them would hide a caller that forgets to set ChecksumMode. + out, err := client.HeadObject(context.TODO(), &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(sha256ChecksumKey), + }) + require.NoError(t, err) + require.Nil(t, out.ChecksumSHA256, + "ChecksumSHA256 should be absent when ChecksumMode is not enabled") + }) + + t.Run("HeadObject returns the stored SHA256 when ChecksumMode is enabled", func(t *testing.T) { + if sha256ChecksumKey == "" { + t.Skip("no object with an SHA256 checksum available in this bucket") + } + out, err := client.HeadObject(context.TODO(), &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(sha256ChecksumKey), + ChecksumMode: s3Types.ChecksumModeEnabled, + }) + require.NoError(t, err) + require.NotNil(t, out.ChecksumSHA256, "ChecksumSHA256 should be present") + require.NotEmpty(t, *out.ChecksumSHA256) + // A full-object checksum is plain Base64. A composite (multipart) one + // carries a "-N" part-count suffix, which is how both this codebase and + // the SDK's own response validation tell them apart. + require.NotContains(t, *out.ChecksumSHA256, "-", + "a single-part upload should carry a full-object checksum") + require.Equal(t, s3Types.ChecksumTypeFullObject, out.ChecksumType) + }) } func TestS3Contract_Fake(t *testing.T) { @@ -123,11 +190,17 @@ func TestS3Contract_Fake(t *testing.T) { "README.md": []byte("# readme\n"), "dummy/dummy_2/template.yml": []byte("key: value\n"), }, + Checksums: map[string]FakeS3Checksum{ + "README.md": { + SHA256: base64Sha256([]byte("# readme\n")), + Type: s3Types.ChecksumTypeFullObject, + }, + }, // One object per page so the pagination contract is genuinely exercised. PageSize: 1, } - runS3ContractTests(t, client, bucket, "README.md") + runS3ContractTests(t, client, bucket, "README.md", "README.md") // Error injection is a fake-specific mechanism with no real-API equivalent. // These tests verify the fake itself, not the contract. @@ -153,6 +226,16 @@ func TestS3Contract_Fake(t *testing.T) { require.Error(t, err) }) + t.Run("HeadObject returns error when HeadObjectErr is injected", func(t *testing.T) { + client.HeadObjectErr = errInjected + defer func() { client.HeadObjectErr = nil }() + _, err := client.HeadObject(context.TODO(), &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String("README.md"), + }) + require.Error(t, err) + }) + // The fake rejects listing inputs outside the contract. Real S3 accepts // these without erroring, so they cannot live in runS3ContractTests — they // exist so a future caller fails loudly instead of hanging or panicking. @@ -184,5 +267,5 @@ func TestS3Contract_RealAWS(t *testing.T) { client, err := defaultNewS3Client(creds) require.NoError(t, err) - runS3ContractTests(t, client, "kosli-cli-public", "README.md") + runS3ContractTests(t, client, "kosli-cli-public", "README.md", "") } From 0f87a32fae8804bdee50e4019d4c2892f0a5a0cd Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Sat, 1 Aug 2026 23:34:55 +0100 Subject: [PATCH 3/7] feat(snapshot s3): add --fingerprint-source to fingerprint from S3 metadata kosli snapshot s3 downloads every matching object into a temp dir and hashes it. For a large bucket that is a full egress, local disk equal to the bucket size, and a SHA256 pass over every byte. --fingerprint-source metadata reads the SHA256 checksum S3 already stores for each object instead, via HeadObject with ChecksumMode enabled. The fingerprint is identical to the downloaded one, so a snapshot still matches the artifact that was attested. This slice covers the one-object case; combining several objects into a directory fingerprint follows, and until then a multi-object selection fails with a message that says how to narrow it. Note what this does not buy: AWS requires s3:GetObject to read an object's checksum, the same permission downloading needs, so the saving is egress, disk and CPU rather than permissions. The flag help says so rather than implying otherwise. Objects must carry a full-object SHA256 checksum. A composite (multipart) checksum hashes the part checksums rather than the object, so it is rejected on both signals S3 gives -- the COMPOSITE type and the "-N" suffix, the latter being what the SDK's own response validation keys off. The error points at copy-object, which collapses a multipart object into a single part in place and needs no access to the original file. Both modes now list through listMatchingS3Objects, so what a filter selects cannot drift between them, and decodeLambdaFingerprint becomes decodeBase64Sha256 now that Lambda is not its only caller. --- cmd/kosli/root.go | 3 + cmd/kosli/snapshotS3.go | 34 ++++- cmd/kosli/snapshotS3_test.go | 44 +++++- internal/aws/aws.go | 84 +++++++---- internal/aws/aws_test.go | 4 +- internal/aws/s3_metadata.go | 139 +++++++++++++++++ internal/aws/s3_metadata_test.go | 249 +++++++++++++++++++++++++++++++ 7 files changed, 515 insertions(+), 42 deletions(-) create mode 100644 internal/aws/s3_metadata.go create mode 100644 internal/aws/s3_metadata_test.go diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 88d8c0d3b..603e48625 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -105,6 +105,8 @@ The ^.kosli_ignore^ will be treated as part of the artifact like any other file, // single source of truth for the env type lists shown in flag help texts; // the server is the authority on which types are actually accepted + validS3FingerprintSources = "content, metadata" + validEnvTypesList = "K8S, ECS, S3, lambda, server, docker, azure-apps, cloud-run, logical" // single source of truth for the service account privilege list shown in @@ -248,6 +250,7 @@ The ^.kosli_ignore^ will be treated as part of the artifact like any other file, bucketPathsRegexFlag = "[optional] The comma separated list of Go regular expressions matched against object keys in the S3 bucket to include when fingerprinting. Cannot be used together with --exclude or --exclude-regex." excludeBucketPathsFlag = "[optional] The comma separated list of file and/or directory paths in the S3 bucket to exclude when fingerprinting. Paths match by literal prefix. Cannot be used together with --include or --include-regex." excludeBucketPathsRegexFlag = "[optional] The comma separated list of Go regular expressions matched against object keys in the S3 bucket to exclude when fingerprinting. Cannot be used together with --include or --include-regex." + s3FingerprintSourceFlag = "[defaulted] How to fingerprint the bucket content. Valid sources are: [" + validS3FingerprintSources + "]. 'content' downloads every matching object and hashes it. 'metadata' reads the SHA256 checksum S3 stores for each object instead, which requires every matching object to have been uploaded with a full-object SHA256 checksum. Both produce the same fingerprint and need the same permissions." pathsFlag = "The comma separated list of absolute or relative paths of artifact directories or files. Can take glob patterns, but be aware that each matching path will be reported as an artifact." excludePathsFlag = "[optional] The comma separated list of directories and files to exclude from fingerprinting. Can take glob patterns. Only applicable for --artifact-type dir." serverExcludePathsFlag = "[optional] The comma separated list of directories and files to exclude from fingerprinting. Can take glob patterns." diff --git a/cmd/kosli/snapshotS3.go b/cmd/kosli/snapshotS3.go index 7ed2670c6..009c62eb1 100644 --- a/cmd/kosli/snapshotS3.go +++ b/cmd/kosli/snapshotS3.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "io" "net/http" "net/url" @@ -68,13 +69,20 @@ kosli snapshot s3 yourEnvironmentName \ --org yourOrgName ` +// fingerprint sources accepted by --fingerprint-source +const ( + fingerprintSourceContent = "content" + fingerprintSourceMetadata = "metadata" +) + type snapshotS3Options struct { - bucket string - includePaths []string - includeRegex []string - excludePaths []string - excludeRegex []string - awsStaticCreds *aws.AWSStaticCreds + bucket string + includePaths []string + includeRegex []string + excludePaths []string + excludeRegex []string + fingerprintSource string + awsStaticCreds *aws.AWSStaticCreds } func newSnapshotS3Cmd(out io.Writer) *cobra.Command { @@ -106,6 +114,12 @@ func newSnapshotS3Cmd(out io.Writer) *cobra.Command { } } + if o.fingerprintSource != fingerprintSourceContent && o.fingerprintSource != fingerprintSourceMetadata { + return ErrorBeforePrintingUsage(cmd, fmt.Sprintf( + "%s is not a valid fingerprint source. Valid sources are: [%s]", + o.fingerprintSource, validS3FingerprintSources)) + } + return nil }, RunE: func(cmd *cobra.Command, args []string) error { @@ -118,6 +132,7 @@ func newSnapshotS3Cmd(out io.Writer) *cobra.Command { cmd.Flags().StringSliceVar(&o.includeRegex, "include-regex", []string{}, bucketPathsRegexFlag) cmd.Flags().StringSliceVarP(&o.excludePaths, "exclude", "x", []string{}, excludeBucketPathsFlag) cmd.Flags().StringSliceVar(&o.excludeRegex, "exclude-regex", []string{}, excludeBucketPathsRegexFlag) + cmd.Flags().StringVar(&o.fingerprintSource, "fingerprint-source", fingerprintSourceContent, s3FingerprintSourceFlag) addAWSAuthFlags(cmd, o.awsStaticCreds) addDryRunFlag(cmd) @@ -141,7 +156,12 @@ func (o *snapshotS3Options) run(args []string) error { return err } - s3Data, err := o.awsStaticCreds.GetS3Data(o.bucket, o.includePaths, o.includeRegex, o.excludePaths, o.excludeRegex, logger) + harvest := o.awsStaticCreds.GetS3Data + if o.fingerprintSource == fingerprintSourceMetadata { + harvest = o.awsStaticCreds.GetS3DataFromMetadata + } + + s3Data, err := harvest(o.bucket, o.includePaths, o.includeRegex, o.excludePaths, o.excludeRegex, logger) if err != nil { return err } diff --git a/cmd/kosli/snapshotS3_test.go b/cmd/kosli/snapshotS3_test.go index b83af80dc..a81a2e36c 100644 --- a/cmd/kosli/snapshotS3_test.go +++ b/cmd/kosli/snapshotS3_test.go @@ -1,9 +1,13 @@ package main import ( + "crypto/sha256" + "encoding/base64" "fmt" "testing" + s3Types "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/kosli-dev/cli/internal/aws" "github.com/stretchr/testify/suite" ) @@ -33,12 +37,22 @@ func (suite *SnapshotS3TestSuite) SetupTest() { // Inject a fake S3 client so tests run without AWS credentials. // The fake is seeded with the objects the test cases filter on. bucketName := suite.bucketName + objects := map[string][]byte{ + "README.md": []byte("# kosli cli public\n"), + "dummy/dummy_2/template.yml": []byte("key: value\n"), + } + // Only README.md carries a stored checksum, so the metadata cases cover both + // an object that can be fingerprinted from metadata and one that cannot. + readmeSum := sha256.Sum256(objects["README.md"]) aws.NewS3ClientFunc = func(_ *aws.AWSStaticCreds) (aws.S3API, error) { return &aws.FakeS3Client{ - Bucket: bucketName, - Objects: map[string][]byte{ - "README.md": []byte("# kosli cli public\n"), - "dummy/dummy_2/template.yml": []byte("key: value\n"), + Bucket: bucketName, + Objects: objects, + Checksums: map[string]aws.FakeS3Checksum{ + "README.md": { + SHA256: base64.StdEncoding.EncodeToString(readmeSum[:]), + Type: s3Types.ChecksumTypeFullObject, + }, }, }, nil } @@ -113,6 +127,28 @@ func (suite *SnapshotS3TestSuite) TestSnapshotS3Cmd() { cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --exclude dummy`, suite.envName, suite.defaultKosliArguments, suite.bucketName), golden: "bucket kosli-cli-public was reported to environment snapshot-s3-env\n", }, + { + name: "--fingerprint-source metadata fingerprints from the stored checksum", + cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --include README.md --fingerprint-source metadata`, suite.envName, suite.defaultKosliArguments, suite.bucketName), + golden: "bucket kosli-cli-public was reported to environment snapshot-s3-env\n", + }, + { + name: "--fingerprint-source content is the default behaviour", + cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --fingerprint-source content`, suite.envName, suite.defaultKosliArguments, suite.bucketName), + golden: "bucket kosli-cli-public was reported to environment snapshot-s3-env\n", + }, + { + wantError: true, + name: "--fingerprint-source rejects an unknown value", + cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --fingerprint-source etag`, suite.envName, suite.defaultKosliArguments, suite.bucketName), + golden: "Error: etag is not a valid fingerprint source. Valid sources are: [content, metadata]\nUsage: kosli snapshot s3 ENVIRONMENT-NAME [flags]\n", + }, + { + wantError: true, + name: "--fingerprint-source metadata fails on an object with no stored checksum", + cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --include dummy --fingerprint-source metadata`, suite.envName, suite.defaultKosliArguments, suite.bucketName), + golden: "Error: object \"dummy/dummy_2/template.yml\" in bucket [kosli-cli-public] has no SHA256 checksum, so its fingerprint cannot be derived from S3 metadata. Re-upload it with a checksum: aws s3api put-object --bucket kosli-cli-public --key dummy/dummy_2/template.yml --body --checksum-algorithm SHA256. Or fingerprint by downloading the objects instead\n", + }, } for _, t := range tests { diff --git a/internal/aws/aws.go b/internal/aws/aws.go index 4470c0691..fdda6b952 100644 --- a/internal/aws/aws.go +++ b/internal/aws/aws.go @@ -384,7 +384,7 @@ func processOneLambdaFunc(lastModified, codeSha256, functionName, packageType st lambdaData.Digests = map[string]string{functionName: codeSha256} if packageType == "Zip" { - lambdaData.Digests[functionName], err = decodeLambdaFingerprint(codeSha256) + lambdaData.Digests[functionName], err = decodeBase64Sha256(codeSha256) if err != nil { return lambdaData, err } @@ -399,8 +399,10 @@ func formatLambdaLastModified(lastModified string) (time.Time, error) { return time.Parse(layout, lastModified) } -// decodeLambdaFingerprint decodes a base64 lambda function fingerprint -func decodeLambdaFingerprint(fingerprint string) (string, error) { +// decodeBase64Sha256 converts a Base64-encoded SHA256 digest into the hex form +// Kosli fingerprints use. AWS reports stored digests in Base64: Lambda's +// CodeSha256 and an S3 object's checksum both arrive this way. +func decodeBase64Sha256(fingerprint string) (string, error) { sha256base64, err := base64.StdEncoding.DecodeString(fingerprint) if err != nil { return "", err @@ -516,40 +518,22 @@ func getS3DataFromClient(client S3API, bucket string, includePaths, includeRegex } }() - params := &s3.ListObjectsV2Input{ - Bucket: aws.String(bucket), + objects, err := listMatchingS3Objects(client, bucket, includePaths, includeRegexCompiled, + excludePaths, excludeRegexCompiled) + if err != nil { + return s3Data, err } var lastModifiedTime *time.Time - paginator := s3.NewListObjectsV2Paginator(client, params) - for paginator.HasMorePages() { - objects, err := paginator.NextPage(context.TODO()) - if err != nil { + for _, object := range objects { + if err := downloadFileFromBucket(client, tempDirName, object.key, bucket, logger); err != nil { return s3Data, err } - - for _, object := range objects.Contents { - if strings.HasSuffix(*object.Key, "/") { // skip folders - continue - } - if shouldExcludePath(*object.Key, includePaths, includeRegexCompiled, excludePaths, excludeRegexCompiled) { - continue - } - err := downloadFileFromBucket(client, tempDirName, *object.Key, bucket, logger) - if err != nil { - return s3Data, err - } - - if lastModifiedTime == nil || object.LastModified.After(*lastModifiedTime) { - lastModifiedTime = object.LastModified - } + if lastModifiedTime == nil || object.lastModified.After(*lastModifiedTime) { + lastModifiedTime = &object.lastModified } } - if lastModifiedTime == nil { - return s3Data, fmt.Errorf("no matching file or dirs in bucket: [%s]", bucket) - } - fileSnapshot, artifactPath, err := containsSingleFile(tempDirName) if err != nil { return s3Data, err @@ -574,6 +558,48 @@ func getS3DataFromClient(client S3API, bucket string, includePaths, includeRegex return s3Data, nil } +// s3Object is a bucket object that passed the include/exclude filters. +type s3Object struct { + key string + lastModified time.Time +} + +// listMatchingS3Objects paginates the bucket and returns the objects that pass +// the filters, skipping the folder markers S3 returns for explicitly created +// folders. It errors when nothing matches, because a snapshot of nothing cannot +// be fingerprinted. +// +// Both fingerprint modes list through here, so what a filter selects cannot +// drift between them. +func listMatchingS3Objects(client S3ListAPI, bucket string, includePaths []string, + includeRegex []*regexp.Regexp, excludePaths []string, excludeRegex []*regexp.Regexp) ([]s3Object, error) { + matched := []s3Object{} + + paginator := s3.NewListObjectsV2Paginator(client, &s3.ListObjectsV2Input{ + Bucket: aws.String(bucket), + }) + for paginator.HasMorePages() { + page, err := paginator.NextPage(context.TODO()) + if err != nil { + return nil, err + } + for _, object := range page.Contents { + if strings.HasSuffix(*object.Key, "/") { // skip folders + continue + } + if shouldExcludePath(*object.Key, includePaths, includeRegex, excludePaths, excludeRegex) { + continue + } + matched = append(matched, s3Object{key: *object.Key, lastModified: *object.LastModified}) + } + } + + if len(matched) == 0 { + return nil, fmt.Errorf("no matching file or dirs in bucket: [%s]", bucket) + } + return matched, nil +} + func downloadFileFromBucket(downloader S3DownloadAPI, dirName, key, bucket string, logger *logger.Logger) error { file, err := utils.CreateFile(filepath.Join(dirName, key)) if err != nil { diff --git a/internal/aws/aws_test.go b/internal/aws/aws_test.go index d6d03654e..030a3c73a 100644 --- a/internal/aws/aws_test.go +++ b/internal/aws/aws_test.go @@ -68,9 +68,9 @@ func (suite *AWSTestSuite) TestDecodeLambdaFingerprint() { }, } { suite.Run(t.name, func() { - got, err := decodeLambdaFingerprint(t.base64Fingerprint) + got, err := decodeBase64Sha256(t.base64Fingerprint) require.False(suite.T(), (err != nil) != t.wantErr, - "decodeLambdaFingerprint() error = %v, wantErr %v", err, t.wantErr) + "decodeBase64Sha256() error = %v, wantErr %v", err, t.wantErr) if !t.wantErr { require.Equal(suite.T(), t.wantFingerprint, got) } diff --git a/internal/aws/s3_metadata.go b/internal/aws/s3_metadata.go new file mode 100644 index 000000000..6f6acf37d --- /dev/null +++ b/internal/aws/s3_metadata.go @@ -0,0 +1,139 @@ +package aws + +import ( + "context" + "fmt" + "strings" + + "github.com/aws/aws-sdk-go-v2/aws" + "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/logger" +) + +// GetS3DataFromMetadata returns a digest and metadata of the S3 bucket content, +// fingerprinting it from the SHA256 checksums S3 stores for each object instead +// of downloading the objects and hashing them. +// +// The fingerprint is identical to the one GetS3Data produces, so a snapshot +// still matches the artifact that was attested. This saves the download, the +// local disk and the local hashing, but not permissions: reading an object's +// checksum needs s3:GetObject, the same permission that downloading it needs. +// +// Every matching object must carry a full-object SHA256 checksum, which S3 only +// stores when the upload asked for one. +func (staticCreds *AWSStaticCreds) GetS3DataFromMetadata(bucket string, includePaths, includeRegex, + excludePaths, excludeRegex []string, logger *logger.Logger) ([]*S3Data, error) { + client, err := NewS3ClientFunc(staticCreds) + if err != nil { + return []*S3Data{}, err + } + return getS3DataFromMetadataClient(client, bucket, includePaths, includeRegex, + excludePaths, excludeRegex, logger) +} + +// getS3DataFromMetadataClient harvests bucket content using the provided client. +// It takes S3MetadataAPI rather than S3API so that it cannot read object content. +func getS3DataFromMetadataClient(client S3MetadataAPI, bucket string, includePaths, includeRegex, + excludePaths, excludeRegex []string, logger *logger.Logger) ([]*S3Data, error) { + s3Data := []*S3Data{} + + includeRegexCompiled, err := compilePathRegex(includeRegex) + if err != nil { + return s3Data, err + } + excludeRegexCompiled, err := compilePathRegex(excludeRegex) + if err != nil { + return s3Data, err + } + + objects, err := listMatchingS3Objects(client, bucket, includePaths, includeRegexCompiled, + excludePaths, excludeRegexCompiled) + if err != nil { + return s3Data, err + } + + // Fingerprinting a whole bucket from metadata needs the digests combined the + // way a directory fingerprint combines them, which is the next slice. + if len(objects) > 1 { + return s3Data, fmt.Errorf( + "fingerprinting %d objects from S3 metadata is not supported yet: narrow the selection to a "+ + "single object with --include or --include-regex, or fingerprint by downloading the objects", + len(objects)) + } + + object := objects[0] + sha256, err := objectSha256FromMetadata(client, bucket, object.key, logger) + if err != nil { + return s3Data, err + } + + // A one-object snapshot is named after the object, matching what + // containsSingleFile + FileSha256 produce when the objects are downloaded. + artifactName := object.key + if index := strings.LastIndex(artifactName, "/"); index >= 0 { + artifactName = artifactName[index+1:] + } + + s3Data = append(s3Data, &S3Data{ + Digests: map[string]string{artifactName: sha256}, + LastModifiedTimestamp: object.lastModified.Unix(), + }) + return s3Data, nil +} + +// objectSha256FromMetadata reads one object's stored SHA256 and returns it as hex. +func objectSha256FromMetadata(client S3HeadAPI, bucket, key string, logger *logger.Logger) (string, error) { + // S3 only returns a stored checksum when the request asks for it. + out, err := client.HeadObject(context.TODO(), &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + ChecksumMode: s3Types.ChecksumModeEnabled, + }) + if err != nil { + return "", fmt.Errorf("failed to read checksum metadata for object %q in bucket [%s]: %w. "+ + "This requires the s3:GetObject permission (the same permission needed to fingerprint by "+ + "downloading); an SSE-KMS object also needs kms:GenerateDataKey and kms:Decrypt", key, bucket, err) + } + + sha256, err := objectChecksumSha256(bucket, key, out) + if err != nil { + return "", err + } + logger.Debug("object %s -- checksum digest: %s", key, sha256) + return sha256, nil +} + +// objectChecksumSha256 converts one HeadObject result into a hex SHA256 of the +// object's content, or explains why it cannot. +func objectChecksumSha256(bucket, key string, out *s3.HeadObjectOutput) (string, error) { + if out.ChecksumSHA256 == nil || *out.ChecksumSHA256 == "" { + return "", fmt.Errorf("object %q in bucket [%s] has no SHA256 checksum, so its fingerprint "+ + "cannot be derived from S3 metadata. Re-upload it with a checksum: "+ + "aws s3api put-object --bucket %s --key %s --body --checksum-algorithm SHA256. "+ + "Or fingerprint by downloading the objects instead", key, bucket, bucket, key) + } + + // A composite checksum hashes the part checksums rather than the object, so + // it is not the object's digest. S3 reports it two ways -- an explicit + // COMPOSITE type, and a "-N" part-count suffix on the value -- and the SDK + // itself treats a "-" as the marker when deciding whether a checksum can be + // validated. Check both, so neither a missing type nor a missing suffix + // lets a composite checksum through. + checksum := *out.ChecksumSHA256 + if out.ChecksumType == s3Types.ChecksumTypeComposite || strings.Contains(checksum, "-") { + return "", fmt.Errorf("object %q in bucket [%s] has a multipart (composite) SHA256 checksum "+ + "%q, which hashes the part checksums rather than the object content. Re-upload it as a "+ + "single part with an SHA256 checksum, or copy it in place to collapse the parts: "+ + "aws s3api copy-object --checksum-algorithm SHA256 --copy-source %s/%s --bucket %s --key %s. "+ + "Or fingerprint by downloading the objects instead", + key, bucket, checksum, bucket, key, bucket, key) + } + + sha256, err := decodeBase64Sha256(checksum) + if err != nil { + return "", fmt.Errorf("failed to decode the SHA256 checksum %q of object %q in bucket [%s]: %w", + checksum, key, bucket, err) + } + return sha256, nil +} diff --git a/internal/aws/s3_metadata_test.go b/internal/aws/s3_metadata_test.go new file mode 100644 index 000000000..d0b8138e4 --- /dev/null +++ b/internal/aws/s3_metadata_test.go @@ -0,0 +1,249 @@ +package aws + +import ( + "testing" + "time" + + s3Types "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/kosli-dev/cli/internal/logger" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type S3MetadataTestSuite struct { + suite.Suite +} + +// fullObjectChecksums builds the checksum map S3 would report for objects +// uploaded single-part with --checksum-algorithm SHA256. +func fullObjectChecksums(objects map[string][]byte) map[string]FakeS3Checksum { + checksums := map[string]FakeS3Checksum{} + for key, content := range objects { + checksums[key] = FakeS3Checksum{ + SHA256: base64Sha256(content), + Type: s3Types.ChecksumTypeFullObject, + } + } + return checksums +} + +func (suite *S3MetadataTestSuite) TestGetS3DataFromMetadataClient() { + earlier := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC) + later := time.Date(2024, 3, 20, 8, 0, 0, 0, time.UTC) + + readme := []byte(fakeReadmeBody) + for _, t := range []struct { + name string + objects map[string][]byte + checksums map[string]FakeS3Checksum + lastModified map[string]time.Time + includePaths []string + excludePaths []string + listErr error + headErr error + wantArtifactName string + wantFingerprint string + wantLastModified int64 + wantErr bool + wantErrMsg string + }{ + { + name: "a single object is fingerprinted from its stored checksum", + objects: map[string][]byte{"README.md": readme}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme}), + wantArtifactName: "README.md", + wantFingerprint: fakeReadmeSha256, + }, + { + name: "a single nested object keeps its base name", + objects: map[string][]byte{"dummy/dummy_2/template.yml": []byte(fakeTemplateBody)}, + checksums: fullObjectChecksums(map[string][]byte{"dummy/dummy_2/template.yml": []byte(fakeTemplateBody)}), + wantArtifactName: "template.yml", + wantFingerprint: fakeTemplateSha256, + }, + { + name: "the newest matched object sets the timestamp", + objects: map[string][]byte{"README.md": readme}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme}), + lastModified: map[string]time.Time{"README.md": later}, + wantArtifactName: "README.md", + wantLastModified: later.Unix(), + }, + { + name: "folder markers are skipped", + objects: map[string][]byte{"dummy/": nil, "README.md": readme}, + checksums: fullObjectChecksums(map[string][]byte{ + "README.md": readme, + }), + lastModified: map[string]time.Time{"README.md": earlier}, + wantArtifactName: "README.md", + wantFingerprint: fakeReadmeSha256, + }, + { + name: "filters select the object to fingerprint", + objects: map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}), + includePaths: []string{"README.md"}, + wantArtifactName: "README.md", + wantFingerprint: fakeReadmeSha256, + }, + { + name: "an object with no stored checksum is an error", + objects: map[string][]byte{"README.md": readme}, + checksums: nil, + wantErr: true, + // The message has to say how to fix it, not just what is wrong. + wantErrMsg: "has no SHA256 checksum", + }, + { + name: "a composite multipart checksum is an error", + objects: map[string][]byte{"README.md": readme}, + checksums: map[string]FakeS3Checksum{ + "README.md": {SHA256: base64Sha256(readme) + "-4", Type: s3Types.ChecksumTypeComposite}, + }, + wantErr: true, + wantErrMsg: "multipart (composite) SHA256 checksum", + }, + { + name: "a composite checksum is rejected on its type even without a suffix", + objects: map[string][]byte{"README.md": readme}, + checksums: map[string]FakeS3Checksum{ + "README.md": {SHA256: base64Sha256(readme), Type: s3Types.ChecksumTypeComposite}, + }, + wantErr: true, + wantErrMsg: "multipart (composite) SHA256 checksum", + }, + { + name: "a checksum that is not valid Base64 is an error", + objects: map[string][]byte{"README.md": readme}, + checksums: map[string]FakeS3Checksum{ + "README.md": {SHA256: "not base64!", Type: s3Types.ChecksumTypeFullObject}, + }, + wantErr: true, + wantErrMsg: "README.md", + }, + { + // Combining several objects into one directory fingerprint is the + // next slice; until then the limit is explicit rather than silent. + name: "more than one object is not supported yet", + objects: map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}), + wantErr: true, + // The message must tell the user how to get a working snapshot. + wantErrMsg: "not supported yet", + }, + { + name: "an empty bucket is an error", + objects: map[string][]byte{"dummy/": nil}, + wantErr: true, + wantErrMsg: "no matching file or dirs in bucket: [" + fakeS3TestBucketName + "]", + }, + { + name: "filtering everything out is an error", + objects: map[string][]byte{"README.md": readme}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme}), + includePaths: []string{"non-existing.md"}, + wantErr: true, + wantErrMsg: "no matching file or dirs in bucket: [" + fakeS3TestBucketName + "]", + }, + { + name: "a listing error propagates", + objects: map[string][]byte{"README.md": readme}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme}), + listErr: errInjected, + wantErr: true, + wantErrMsg: "injected error", + }, + { + name: "a metadata error propagates", + objects: map[string][]byte{"README.md": readme}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme}), + headErr: errInjected, + wantErr: true, + wantErrMsg: "injected error", + }, + } { + suite.Run(t.name, func() { + client := &FakeS3Client{ + Bucket: fakeS3TestBucketName, + Objects: t.objects, + Checksums: t.checksums, + LastModified: t.lastModified, + ListObjectsV2Err: t.listErr, + HeadObjectErr: t.headErr, + } + + data, err := getS3DataFromMetadataClient(client, fakeS3TestBucketName, t.includePaths, + nil, t.excludePaths, nil, logger.NewStandardLogger()) + + if t.wantErr { + require.Error(suite.T(), err) + if t.wantErrMsg != "" { + require.Contains(suite.T(), err.Error(), t.wantErrMsg) + } + return + } + require.NoError(suite.T(), err) + require.Len(suite.T(), data, 1) + + wantArtifactName := t.wantArtifactName + if wantArtifactName == "" { + wantArtifactName = fakeS3TestBucketName + } + require.Contains(suite.T(), data[0].Digests, wantArtifactName) + if t.wantFingerprint != "" { + require.Equal(suite.T(), t.wantFingerprint, data[0].Digests[wantArtifactName]) + } + if t.wantLastModified != 0 { + require.Equal(suite.T(), t.wantLastModified, data[0].LastModifiedTimestamp) + } + }) + } +} + +// TestMetadataMatchesDownloadFingerprint is the test the whole feature rests on: +// for the same bucket, fingerprinting from stored checksums must produce exactly +// what downloading and hashing produces. If these ever diverge, a snapshot stops +// matching the artifact that was attested. +func (suite *S3MetadataTestSuite) TestMetadataMatchesDownloadFingerprint() { + for _, t := range []struct { + name string + objects map[string][]byte + }{ + { + name: "a single object", + objects: map[string][]byte{"README.md": []byte(fakeReadmeBody)}, + }, + { + name: "a single nested object", + objects: map[string][]byte{ + "dummy/dummy_2/template.yml": []byte(fakeTemplateBody), + }, + }, + } { + suite.Run(t.name, func() { + newClient := func() *FakeS3Client { + return &FakeS3Client{ + Bucket: fakeS3TestBucketName, + Objects: t.objects, + Checksums: fullObjectChecksums(t.objects), + } + } + + downloaded, err := getS3DataFromClient(newClient(), fakeS3TestBucketName, + nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + fromMetadata, err := getS3DataFromMetadataClient(newClient(), fakeS3TestBucketName, + nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + require.Equal(suite.T(), downloaded, fromMetadata, + "metadata mode must produce the same artifacts as content mode") + }) + } +} + +func TestS3MetadataTestSuite(t *testing.T) { + suite.Run(t, new(S3MetadataTestSuite)) +} From 26a4d7cd6080202392024009294793aa39100fee Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Sat, 1 Aug 2026 23:34:56 +0100 Subject: [PATCH 4/7] feat(snapshot s3): fingerprint multi-object buckets from metadata Combine the per-object checksums with digest.VirtualDirSha256, which reproduces the directory fingerprint the download path produces, so --fingerprint-source metadata now covers a whole bucket rather than a single object. One object is still fingerprinted as that file and named after it; several are fingerprinted as a directory named after the bucket, matching what containsSingleFile decides once the objects are on disk. A bucket-root .kosli_ignore is rejected rather than ignored. Its rules change the fingerprint, and applying them would mean downloading the file. Silently skipping them would produce a fingerprint that quietly differs from the downloaded one, which is worse than refusing. Only a root one matters: DirSha256 reads exactly one, at the artifact root, and a nested .kosli_ignore is an ordinary file to it. A lone .kosli_ignore is fine, because a one-object snapshot is fingerprinted as a file and the ignore rules never come into play. The equality tests now cover multi-object trees, including a prefix that shares a name prefix with a sibling object -- the case where sorting the flat key list instead of the tree would diverge, because '.' sorts before '/' in a byte-ordered listing. --- internal/aws/s3_metadata.go | 75 +++++++++++++++++++++++------- internal/aws/s3_metadata_test.go | 80 +++++++++++++++++++++++++++++--- 2 files changed, 131 insertions(+), 24 deletions(-) diff --git a/internal/aws/s3_metadata.go b/internal/aws/s3_metadata.go index 6f6acf37d..f12f5e6b7 100644 --- a/internal/aws/s3_metadata.go +++ b/internal/aws/s3_metadata.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "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" ) @@ -53,35 +54,75 @@ func getS3DataFromMetadataClient(client S3MetadataAPI, bucket string, includePat return s3Data, err } - // Fingerprinting a whole bucket from metadata needs the digests combined the - // way a directory fingerprint combines them, which is the next slice. - if len(objects) > 1 { - return s3Data, fmt.Errorf( - "fingerprinting %d objects from S3 metadata is not supported yet: narrow the selection to a "+ - "single object with --include or --include-regex, or fingerprint by downloading the objects", - len(objects)) + if err := rejectKosliIgnore(objects, bucket); err != nil { + return s3Data, err } - object := objects[0] - sha256, err := objectSha256FromMetadata(client, bucket, object.key, logger) - if err != nil { - return s3Data, err + files := make([]digest.VirtualFile, 0, len(objects)) + newest := objects[0].lastModified + for _, object := range objects { + sha256, err := objectSha256FromMetadata(client, bucket, object.key, logger) + if err != nil { + return s3Data, err + } + files = append(files, digest.VirtualFile{Path: object.key, Sha256: sha256}) + if object.lastModified.After(newest) { + newest = object.lastModified + } } - // A one-object snapshot is named after the object, matching what - // containsSingleFile + FileSha256 produce when the objects are downloaded. - artifactName := object.key - if index := strings.LastIndex(artifactName, "/"); index >= 0 { - artifactName = artifactName[index+1:] + // One object is fingerprinted as that file and named after it; several are + // fingerprinted as a directory named after the bucket. This mirrors what + // containsSingleFile decides once the objects are on disk. + artifactName := bucket + var sha256 string + if file, ok := digest.SingleVirtualFile(files); ok { + artifactName = file.Name() + sha256 = file.Sha256 + } else { + sha256, err = digest.VirtualDirSha256(files, logger) + if err != nil { + return s3Data, fmt.Errorf("failed to fingerprint bucket [%s] from object metadata: %w", bucket, err) + } } s3Data = append(s3Data, &S3Data{ Digests: map[string]string{artifactName: sha256}, - LastModifiedTimestamp: object.lastModified.Unix(), + LastModifiedTimestamp: newest.Unix(), }) return s3Data, nil } +// kosliIgnoreFile is read from the root of a directory artifact by +// digest.DirSha256, and its rules change the fingerprint. +const kosliIgnoreFile = ".kosli_ignore" + +// rejectKosliIgnore fails when the selection contains a bucket-root +// .kosli_ignore. Applying its rules needs the object's content, which metadata +// mode does not read, and ignoring them would silently produce a fingerprint +// that differs from the downloaded one. +// +// Only a root .kosli_ignore matters: DirSha256 reads exactly one, at the root of +// the artifact, and treats any nested one as an ordinary file. +func rejectKosliIgnore(objects []s3Object, bucket string) error { + for _, object := range objects { + if object.key != kosliIgnoreFile { + continue + } + if len(objects) == 1 { + // The only object: it is fingerprinted as a file, and DirSha256's + // ignore handling never comes into play. + return nil + } + return fmt.Errorf("bucket [%s] has a %s object at its root, and its exclusion rules change the "+ + "fingerprint. Fingerprinting from S3 metadata cannot apply them, because reading the file "+ + "would mean downloading it. Fingerprint by downloading the objects instead. Excluding the "+ + "file with --exclude would not help: the fingerprint would still differ, because the rules "+ + "inside it would go unapplied", bucket, kosliIgnoreFile) + } + return nil +} + // objectSha256FromMetadata reads one object's stored SHA256 and returns it as hex. func objectSha256FromMetadata(client S3HeadAPI, bucket, key string, logger *logger.Logger) (string, error) { // S3 only returns a stored checksum when the request asks for it. diff --git a/internal/aws/s3_metadata_test.go b/internal/aws/s3_metadata_test.go index d0b8138e4..2fbfb1666 100644 --- a/internal/aws/s3_metadata_test.go +++ b/internal/aws/s3_metadata_test.go @@ -123,14 +123,49 @@ func (suite *S3MetadataTestSuite) TestGetS3DataFromMetadataClient() { wantErrMsg: "README.md", }, { - // Combining several objects into one directory fingerprint is the - // next slice; until then the limit is explicit rather than silent. - name: "more than one object is not supported yet", - objects: map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}, - checksums: fullObjectChecksums(map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}), + name: "several objects are fingerprinted as a directory named after the bucket", + objects: map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}, + checksums: fullObjectChecksums(map[string][]byte{"README.md": readme, "notes.txt": []byte(fakeNotesBody)}), + lastModified: map[string]time.Time{"README.md": earlier, "notes.txt": later}, + wantArtifactName: fakeS3TestBucketName, + wantLastModified: later.Unix(), + }, + { + // Reading it would need the object's content, so metadata mode + // cannot apply its rules and must not silently ignore them. + name: "a root .kosli_ignore is an error", + objects: map[string][]byte{ + "README.md": readme, + ".kosli_ignore": []byte("notes.txt\n"), + "notes.txt": []byte(fakeNotesBody), + }, + checksums: fullObjectChecksums(map[string][]byte{ + "README.md": readme, + ".kosli_ignore": []byte("notes.txt\n"), + "notes.txt": []byte(fakeNotesBody), + }), + wantErr: true, + wantErrMsg: ".kosli_ignore", + }, + { + name: "a nested .kosli_ignore is an ordinary object", + objects: map[string][]byte{ + "README.md": readme, + "dummy/.kosli_ignore": []byte("README.md\n"), + }, + checksums: fullObjectChecksums(map[string][]byte{ + "README.md": readme, + "dummy/.kosli_ignore": []byte("README.md\n"), + }), + wantArtifactName: fakeS3TestBucketName, + }, + { + name: "an object key that is also a directory prefix is an error", + objects: map[string][]byte{"a": readme, "a/b": []byte(fakeNotesBody)}, + checksums: fullObjectChecksums(map[string][]byte{"a": readme, "a/b": []byte(fakeNotesBody)}), wantErr: true, - // The message must tell the user how to get a working snapshot. - wantErrMsg: "not supported yet", + // digest rejects it; the message must name the clashing path. + wantErrMsg: "both a file and a directory", }, { name: "an empty bucket is an error", @@ -220,6 +255,37 @@ func (suite *S3MetadataTestSuite) TestMetadataMatchesDownloadFingerprint() { "dummy/dummy_2/template.yml": []byte(fakeTemplateBody), }, }, + { + name: "two objects at the bucket root", + objects: map[string][]byte{ + "README.md": []byte(fakeReadmeBody), + "notes.txt": []byte(fakeNotesBody), + }, + }, + { + name: "objects nested under prefixes", + objects: map[string][]byte{ + "README.md": []byte(fakeReadmeBody), + "dummy/dummy_2/template.yml": []byte(fakeTemplateBody), + "dummy/notes.txt": []byte(fakeNotesBody), + }, + }, + { + // A lone .kosli_ignore is fingerprinted as a file, so its rules + // never apply and metadata mode can handle it like any object. + name: "a lone .kosli_ignore object", + objects: map[string][]byte{".kosli_ignore": []byte("notes.txt\n")}, + }, + { + // '.' sorts before '/', so a flat key sort would order these + // differently from the directory walk the download path uses. + name: "a prefix sharing a name prefix with a sibling object", + objects: map[string][]byte{ + "a.txt": []byte(fakeReadmeBody), + "a/z.txt": []byte(fakeNotesBody), + "b.txt": []byte(fakeTemplateBody), + }, + }, } { suite.Run(t.name, func() { newClient := func() *FakeS3Client { From 0f21e448efc6569b8389937205a8c9f5005002eb Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Sat, 1 Aug 2026 23:34:58 +0100 Subject: [PATCH 5/7] perf(snapshot s3): read object metadata concurrently Metadata mode made one HeadObject call per object, in sequence, so a thousand-object bucket paid a thousand round trips end to end. Fetch them concurrently instead, bounded by a semaphore. Unlike GetLambdaPackageData, which spawns one goroutine per function, this cannot be unbounded: an account holds tens of functions but a bucket can hold millions of objects. 16 in flight stays well under S3's per-prefix request ceiling, and the adaptive retryer already spreads throttling across the batch. Results go into preallocated slots rather than an appended slice, so the digest order follows the listing however the requests interleave. The same bucket then always fingerprints to the same value, and a failure is reproducible rather than depending on which goroutine won. Errors are split by what the user can do about them. A transport or permission failure aborts immediately, since every remaining request would fail the same way. An object whose checksum is missing or composite is collected instead, and one run reports every object that needs fixing -- capped at ten keys plus a count -- so migrating a bucket is not a guess-and-retry loop. --- internal/aws/s3_metadata.go | 159 ++++++++++++---- internal/aws/s3_metadata_concurrency_test.go | 183 +++++++++++++++++++ 2 files changed, 311 insertions(+), 31 deletions(-) create mode 100644 internal/aws/s3_metadata_concurrency_test.go diff --git a/internal/aws/s3_metadata.go b/internal/aws/s3_metadata.go index f12f5e6b7..0aaeebc1f 100644 --- a/internal/aws/s3_metadata.go +++ b/internal/aws/s3_metadata.go @@ -2,8 +2,11 @@ package aws import ( "context" + "errors" "fmt" + "sort" "strings" + "sync" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -58,14 +61,14 @@ func getS3DataFromMetadataClient(client S3MetadataAPI, bucket string, includePat return s3Data, err } - files := make([]digest.VirtualFile, 0, len(objects)) + logger.Debug("reading checksum metadata for %d objects in bucket [%s]", len(objects), bucket) + files, err := fetchObjectChecksums(client, bucket, objects, logger) + if err != nil { + return s3Data, err + } + newest := objects[0].lastModified for _, object := range objects { - sha256, err := objectSha256FromMetadata(client, bucket, object.key, logger) - if err != nil { - return s3Data, err - } - files = append(files, digest.VirtualFile{Path: object.key, Sha256: sha256}) if object.lastModified.After(newest) { newest = object.lastModified } @@ -93,9 +96,125 @@ func getS3DataFromMetadataClient(client S3MetadataAPI, bucket string, includePat return s3Data, nil } -// kosliIgnoreFile is read from the root of a directory artifact by -// digest.DirSha256, and its rules change the fingerprint. -const kosliIgnoreFile = ".kosli_ignore" +const ( + // kosliIgnoreFile is read from the root of a directory artifact by + // digest.DirSha256, and its rules change the fingerprint. + kosliIgnoreFile = ".kosli_ignore" + + // defaultS3MetadataConcurrency bounds the in-flight metadata requests. A + // bucket can hold far more objects than a Lambda account holds functions, + // so unlike GetLambdaPackageData this cannot spawn one goroutine per item. + // 16 stays well under S3's per-prefix request ceiling, and the adaptive + // retryer in NewAWSConfigFromEnvOrFlags already spreads throttling across + // the whole batch. + defaultS3MetadataConcurrency = 16 + + // maxReportedUnusableObjects caps how many keys an error lists before + // summarising the rest, so a bucket-wide problem stays readable. + maxReportedUnusableObjects = 10 +) + +// fetchObjectChecksums reads every object's stored SHA256 concurrently. +// +// Errors are split in two. A transport or permission failure aborts the run at +// once, because every remaining request would fail the same way. An object +// whose checksum cannot be used is collected instead, so one run tells the user +// about every object they need to fix rather than one per attempt. +func fetchObjectChecksums(client S3HeadAPI, bucket string, objects []s3Object, + logger *logger.Logger) ([]digest.VirtualFile, error) { + var ( + wg sync.WaitGroup + mutex sync.Mutex + unusable []error + semaphore = make(chan struct{}, defaultS3MetadataConcurrency) + ) + + // Writing into a preallocated slot keeps the result in listing order + // however the requests interleave, so the same bucket always fingerprints + // the same way and a failure is reproducible. + files := make([]digest.VirtualFile, len(objects)) + + apiErrs := make(chan error, 1) // buffered for the first error only + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + for i, object := range objects { + wg.Add(1) + go func(index int, key string) { + defer wg.Done() + select { + case <-ctx.Done(): + return // another goroutine hit an API error + default: + } + + semaphore <- struct{}{} + defer func() { <-semaphore }() + + out, err := client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + // S3 only returns a stored checksum when the request asks. + ChecksumMode: s3Types.ChecksumModeEnabled, + }) + if err != nil { + select { + case apiErrs <- fmt.Errorf("failed to read checksum metadata for object %q in bucket "+ + "[%s]: %w. This requires the s3:GetObject permission (the same permission needed to "+ + "fingerprint by downloading); an SSE-KMS object also needs kms:GenerateDataKey and "+ + "kms:Decrypt", key, bucket, err): + cancel() + default: // an error is already recorded + } + return + } + + sha256, err := objectChecksumSha256(bucket, key, out) + if err != nil { + mutex.Lock() + unusable = append(unusable, err) + mutex.Unlock() + return + } + logger.Debug("object %s -- checksum digest: %s", key, sha256) + files[index] = digest.VirtualFile{Path: key, Sha256: sha256} + }(i, object.key) + } + + wg.Wait() + close(apiErrs) + if err := <-apiErrs; err != nil { + return nil, err + } + if len(unusable) > 0 { + return nil, combineUnusableObjectErrors(unusable) + } + return files, nil +} + +// combineUnusableObjectErrors reports every object that cannot be fingerprinted, +// capped so a whole-bucket problem stays readable. +func combineUnusableObjectErrors(errs []error) error { + // The goroutines finish in any order; sorting keeps the message stable. + messages := make([]string, 0, len(errs)) + for _, err := range errs { + messages = append(messages, err.Error()) + } + sort.Strings(messages) + + if len(messages) == 1 { + return errors.New(messages[0]) + } + + shown := messages + suffix := "" + if len(shown) > maxReportedUnusableObjects { + shown = shown[:maxReportedUnusableObjects] + suffix = fmt.Sprintf("\n(and %d more)", len(messages)-maxReportedUnusableObjects) + } + return fmt.Errorf("%d objects cannot be fingerprinted from S3 metadata:\n%s%s", + len(messages), strings.Join(shown, "\n"), suffix) +} // rejectKosliIgnore fails when the selection contains a bucket-root // .kosli_ignore. Applying its rules needs the object's content, which metadata @@ -123,28 +242,6 @@ func rejectKosliIgnore(objects []s3Object, bucket string) error { return nil } -// objectSha256FromMetadata reads one object's stored SHA256 and returns it as hex. -func objectSha256FromMetadata(client S3HeadAPI, bucket, key string, logger *logger.Logger) (string, error) { - // S3 only returns a stored checksum when the request asks for it. - out, err := client.HeadObject(context.TODO(), &s3.HeadObjectInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), - ChecksumMode: s3Types.ChecksumModeEnabled, - }) - if err != nil { - return "", fmt.Errorf("failed to read checksum metadata for object %q in bucket [%s]: %w. "+ - "This requires the s3:GetObject permission (the same permission needed to fingerprint by "+ - "downloading); an SSE-KMS object also needs kms:GenerateDataKey and kms:Decrypt", key, bucket, err) - } - - sha256, err := objectChecksumSha256(bucket, key, out) - if err != nil { - return "", err - } - logger.Debug("object %s -- checksum digest: %s", key, sha256) - return sha256, nil -} - // objectChecksumSha256 converts one HeadObject result into a hex SHA256 of the // object's content, or explains why it cannot. func objectChecksumSha256(bucket, key string, out *s3.HeadObjectOutput) (string, error) { diff --git a/internal/aws/s3_metadata_concurrency_test.go b/internal/aws/s3_metadata_concurrency_test.go new file mode 100644 index 000000000..d808c8d61 --- /dev/null +++ b/internal/aws/s3_metadata_concurrency_test.go @@ -0,0 +1,183 @@ +package aws + +import ( + "context" + "fmt" + "strings" + "sync/atomic" + "testing" + + "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/logger" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type S3MetadataConcurrencyTestSuite struct { + suite.Suite +} + +// countingS3Client wraps FakeS3Client to record how many HeadObject calls are +// made and how many run at once. +type countingS3Client struct { + *FakeS3Client + calls atomic.Int64 + inFlight atomic.Int64 + maxInFlight atomic.Int64 + block chan struct{} // when non-nil, HeadObject waits for a send +} + +func (c *countingS3Client) HeadObject(ctx context.Context, params *s3.HeadObjectInput, + optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error) { + c.calls.Add(1) + inFlight := c.inFlight.Add(1) + for { + max := c.maxInFlight.Load() + if inFlight <= max || c.maxInFlight.CompareAndSwap(max, inFlight) { + break + } + } + if c.block != nil { + <-c.block + } + defer c.inFlight.Add(-1) + return c.FakeS3Client.HeadObject(ctx, params, optFns...) +} + +// manyObjects builds a bucket of n checksum-bearing objects. +func manyObjects(n int) (map[string][]byte, map[string]FakeS3Checksum) { + objects := map[string][]byte{} + for i := 0; i < n; i++ { + objects[fmt.Sprintf("object-%03d.txt", i)] = []byte(fmt.Sprintf("content %d\n", i)) + } + return objects, fullObjectChecksums(objects) +} + +func (suite *S3MetadataConcurrencyTestSuite) newClient(n int) *countingS3Client { + objects, checksums := manyObjects(n) + return &countingS3Client{ + FakeS3Client: &FakeS3Client{ + Bucket: fakeS3TestBucketName, + Objects: objects, + Checksums: checksums, + }, + } +} + +func (suite *S3MetadataConcurrencyTestSuite) TestMakesExactlyOneCallPerObject() { + client := suite.newClient(50) + + _, err := getS3DataFromMetadataClient(client, fakeS3TestBucketName, nil, nil, nil, nil, + logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Equal(suite.T(), int64(50), client.calls.Load()) +} + +func (suite *S3MetadataConcurrencyTestSuite) TestRespectsTheConcurrencyBound() { + client := suite.newClient(200) + + _, err := getS3DataFromMetadataClient(client, fakeS3TestBucketName, nil, nil, nil, nil, + logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.LessOrEqual(suite.T(), client.maxInFlight.Load(), int64(defaultS3MetadataConcurrency), + "more requests were in flight than the concurrency bound allows") +} + +// TestFingerprintIsIndependentOfCompletionOrder pins that concurrency cannot +// reorder the digests: the same bucket must fingerprint identically whether the +// requests finish in listing order or not. +func (suite *S3MetadataConcurrencyTestSuite) TestFingerprintIsIndependentOfCompletionOrder() { + first, err := getS3DataFromMetadataClient(suite.newClient(30), fakeS3TestBucketName, + nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + + for i := 0; i < 5; i++ { + again, err := getS3DataFromMetadataClient(suite.newClient(30), fakeS3TestBucketName, + nil, nil, nil, nil, logger.NewStandardLogger()) + require.NoError(suite.T(), err) + require.Equal(suite.T(), first, again) + } +} + +func (suite *S3MetadataConcurrencyTestSuite) TestAnAPIErrorStopsRemainingWork() { + client := suite.newClient(500) + client.HeadObjectErr = errInjected + + _, err := getS3DataFromMetadataClient(client, fakeS3TestBucketName, nil, nil, nil, nil, + logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "injected error") + require.Less(suite.T(), client.calls.Load(), int64(500), + "an API error should cancel the remaining requests rather than running all of them") +} + +// TestReportsEveryUnusableObject checks the error names all the offending keys +// rather than only the first, so a bucket-wide migration is not a +// guess-and-retry loop. +func (suite *S3MetadataConcurrencyTestSuite) TestReportsEveryUnusableObject() { + objects, checksums := manyObjects(6) + // Three objects have no stored checksum. + for _, key := range []string{"object-000.txt", "object-002.txt", "object-004.txt"} { + delete(checksums, key) + } + client := &FakeS3Client{ + Bucket: fakeS3TestBucketName, + Objects: objects, + Checksums: checksums, + } + + _, err := getS3DataFromMetadataClient(client, fakeS3TestBucketName, nil, nil, nil, nil, + logger.NewStandardLogger()) + require.Error(suite.T(), err) + for _, key := range []string{"object-000.txt", "object-002.txt", "object-004.txt"} { + require.Contains(suite.T(), err.Error(), key) + } + require.NotContains(suite.T(), err.Error(), "object-001.txt", + "objects that are fine should not be named") +} + +// TestCapsTheListOfUnusableObjects keeps the error readable when a whole bucket +// is unusable. +func (suite *S3MetadataConcurrencyTestSuite) TestCapsTheListOfUnusableObjects() { + objects, _ := manyObjects(40) + client := &FakeS3Client{ + Bucket: fakeS3TestBucketName, + Objects: objects, + } + + _, err := getS3DataFromMetadataClient(client, fakeS3TestBucketName, nil, nil, nil, nil, + logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "and 30 more") + // Each message names the key once in quotes and once in the fix command, so + // count the quoted form to count messages. + require.Equal(suite.T(), maxReportedUnusableObjects, strings.Count(err.Error(), `"object-`), + "the error should list exactly the capped number of keys") +} + +// TestMixedFailuresReportChecksumProblems checks that when objects are unusable +// for different reasons, both reasons reach the user. +func (suite *S3MetadataConcurrencyTestSuite) TestMixedFailuresReportChecksumProblems() { + objects, checksums := manyObjects(4) + delete(checksums, "object-000.txt") + checksums["object-001.txt"] = FakeS3Checksum{ + SHA256: base64Sha256(objects["object-001.txt"]) + "-3", + Type: s3Types.ChecksumTypeComposite, + } + client := &FakeS3Client{ + Bucket: fakeS3TestBucketName, + Objects: objects, + Checksums: checksums, + } + + _, err := getS3DataFromMetadataClient(client, fakeS3TestBucketName, nil, nil, nil, nil, + logger.NewStandardLogger()) + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "no SHA256 checksum") + require.Contains(suite.T(), err.Error(), "multipart (composite)") +} + +func TestS3MetadataConcurrencyTestSuite(t *testing.T) { + suite.Run(t, new(S3MetadataConcurrencyTestSuite)) +} From 2c051791730d5f702f1abade47413c58de212953 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Sat, 1 Aug 2026 23:34:59 +0100 Subject: [PATCH 6/7] docs(snapshot s3): document the metadata fingerprint source Spell out what --fingerprint-source metadata does and, more importantly, what it does not do. The obvious assumption is that reading metadata instead of object content needs weaker permissions; AWS requires s3:GetObject either way, so the help says so plainly rather than letting readers infer a benefit that is not there. The three conditions that make a bucket unusable in this mode -- objects without a stored checksum, composite multipart checksums, and a root .kosli_ignore -- are documented alongside the fix for each, so a reader can tell before running whether their bucket qualifies. --- cmd/kosli/snapshotS3.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cmd/kosli/snapshotS3.go b/cmd/kosli/snapshotS3.go index 009c62eb1..b0bc93301 100644 --- a/cmd/kosli/snapshotS3.go +++ b/cmd/kosli/snapshotS3.go @@ -17,6 +17,15 @@ 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. +By default the bucket content is fingerprinted by downloading every matching object and hashing it. ^--fingerprint-source metadata^ reads the SHA256 checksum S3 stores for each object instead, which avoids the download, the temporary disk space and the local hashing. Both sources produce the same fingerprint, so a snapshot matches the artifact you attested either way. + +Fingerprinting from metadata comes with three conditions: +- Every matching object must carry a full-object SHA256 checksum. S3 only stores one when the upload asked for it, for example ^aws s3api put-object --checksum-algorithm SHA256^. Objects uploaded without one are reported, and cannot be fingerprinted this way. +- A multipart upload gets a composite SHA256, which hashes the checksums of the individual parts rather than the object content, so it cannot be used as the object's fingerprint. Such an object can be collapsed into a single part in place with ^aws s3api copy-object --checksum-algorithm SHA256 --copy-source yourBucket/yourKey --bucket yourBucket --key yourKey^. +- ^.kosli_ignore^ is not applied, because reading it would mean downloading it. A bucket with a ^.kosli_ignore^ at its root is reported rather than fingerprinted without its rules. + +It does not reduce the permissions the command needs: AWS requires ^s3:GetObject^ to read an object's checksum, the same permission that downloading it needs. Reading the checksum of an SSE-KMS encrypted object additionally needs ^kms:GenerateDataKey^ and ^kms:Decrypt^. + ` + kosliIgnoreDesc const snapshotS3Example = ` @@ -67,6 +76,14 @@ kosli snapshot s3 yourEnvironmentName \ --exclude-regex '.*\.png$' \ --api-token yourAPIToken \ --org yourOrgName + +# report contents of an AWS S3 bucket without downloading the objects, +# using the SHA256 checksums S3 stores for them: +kosli snapshot s3 yourEnvironmentName \ + --bucket yourBucketName \ + --fingerprint-source metadata \ + --api-token yourAPIToken \ + --org yourOrgName ` // fingerprint sources accepted by --fingerprint-source From f7e23d28cbbcf397d49c6cd9f89dfa2f9f5ff910 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Sat, 1 Aug 2026 23:35:00 +0100 Subject: [PATCH 7/7] fix(snapshot s3): document unclean keys and correct the .kosli_ignore error Two points from review. Unusual but legal object keys -- "a//b", or a leading-slash "/foo" -- are rejected by metadata mode while the content source quietly collapses them via filepath.Join, where "a//b" can even land on top of a real "a/b" object. Refusing beats a silently wrong fingerprint, so the behaviour stays, but a bucket that content mode appears to handle will now fail loudly under metadata. Say so in the help text, and cover both key shapes in the tests so the documented behaviour is pinned rather than described. The .kosli_ignore error also over-claimed. It said excluding the file with --exclude "would not help: the fingerprint would still differ", which is not true of the comparison that matters: with the file excluded, neither source applies its rules, so the two agree. What actually differs is that the resulting fingerprint covers the unfiltered bucket content rather than the content the rules were written to select. The message now says that, and an equality test pins the claim instead of leaving it as prose. --- cmd/kosli/snapshotS3.go | 3 ++- internal/aws/s3_metadata.go | 8 +++--- internal/aws/s3_metadata_test.go | 43 +++++++++++++++++++++++++++++--- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/cmd/kosli/snapshotS3.go b/cmd/kosli/snapshotS3.go index b0bc93301..36e394f55 100644 --- a/cmd/kosli/snapshotS3.go +++ b/cmd/kosli/snapshotS3.go @@ -19,10 +19,11 @@ In all cases, the content is reported as one artifact. If you wish to report sep By default the bucket content is fingerprinted by downloading every matching object and hashing it. ^--fingerprint-source metadata^ reads the SHA256 checksum S3 stores for each object instead, which avoids the download, the temporary disk space and the local hashing. Both sources produce the same fingerprint, so a snapshot matches the artifact you attested either way. -Fingerprinting from metadata comes with three conditions: +Fingerprinting from metadata comes with four conditions: - Every matching object must carry a full-object SHA256 checksum. S3 only stores one when the upload asked for it, for example ^aws s3api put-object --checksum-algorithm SHA256^. Objects uploaded without one are reported, and cannot be fingerprinted this way. - A multipart upload gets a composite SHA256, which hashes the checksums of the individual parts rather than the object content, so it cannot be used as the object's fingerprint. Such an object can be collapsed into a single part in place with ^aws s3api copy-object --checksum-algorithm SHA256 --copy-source yourBucket/yourKey --bucket yourBucket --key yourKey^. - ^.kosli_ignore^ is not applied, because reading it would mean downloading it. A bucket with a ^.kosli_ignore^ at its root is reported rather than fingerprinted without its rules. +- Object keys must be clean relative paths. S3 allows keys such as ^a//b^ or ^/foo^, which cannot be mapped onto a directory tree unambiguously — ^a//b^ and ^a/b^ would collide. Such keys are reported rather than fingerprinted; the default ^content^ source silently collapses them instead. It does not reduce the permissions the command needs: AWS requires ^s3:GetObject^ to read an object's checksum, the same permission that downloading it needs. Reading the checksum of an SSE-KMS encrypted object additionally needs ^kms:GenerateDataKey^ and ^kms:Decrypt^. diff --git a/internal/aws/s3_metadata.go b/internal/aws/s3_metadata.go index 0aaeebc1f..5af9b7a16 100644 --- a/internal/aws/s3_metadata.go +++ b/internal/aws/s3_metadata.go @@ -235,9 +235,11 @@ func rejectKosliIgnore(objects []s3Object, bucket string) error { } return fmt.Errorf("bucket [%s] has a %s object at its root, and its exclusion rules change the "+ "fingerprint. Fingerprinting from S3 metadata cannot apply them, because reading the file "+ - "would mean downloading it. Fingerprint by downloading the objects instead. Excluding the "+ - "file with --exclude would not help: the fingerprint would still differ, because the rules "+ - "inside it would go unapplied", bucket, kosliIgnoreFile) + "would mean downloading it. To apply them, fingerprint by downloading the objects instead. "+ + "Excluding the file with --exclude %s is also consistent -- neither source then applies its "+ + "rules, so both produce the same fingerprint -- but that fingerprint covers the unfiltered "+ + "bucket content, not the content the rules were written to select", + bucket, kosliIgnoreFile, kosliIgnoreFile) } return nil } diff --git a/internal/aws/s3_metadata_test.go b/internal/aws/s3_metadata_test.go index 2fbfb1666..52087b717 100644 --- a/internal/aws/s3_metadata_test.go +++ b/internal/aws/s3_metadata_test.go @@ -167,6 +167,26 @@ func (suite *S3MetadataTestSuite) TestGetS3DataFromMetadataClient() { // digest rejects it; the message must name the clashing path. wantErrMsg: "both a file and a directory", }, + { + // S3 allows these keys, but they cannot be mapped onto a directory + // tree unambiguously: content mode silently collapses "a//b" onto + // "a/b" via filepath.Join, which can even collide with a real + // "a/b" object. Refusing beats a quietly wrong fingerprint, and + // the help text says so. + name: "an object key with an empty path segment is an error", + objects: map[string][]byte{"a//b": readme, "c.txt": []byte(fakeNotesBody)}, + checksums: fullObjectChecksums(map[string][]byte{"a//b": readme, "c.txt": []byte(fakeNotesBody)}), + wantErr: true, + // The wrapper must name the bucket and keep the underlying reason. + wantErrMsg: "not a clean relative path", + }, + { + name: "an absolute object key is an error", + objects: map[string][]byte{"/foo": readme, "c.txt": []byte(fakeNotesBody)}, + checksums: fullObjectChecksums(map[string][]byte{"/foo": readme, "c.txt": []byte(fakeNotesBody)}), + wantErr: true, + wantErrMsg: "not a clean relative path", + }, { name: "an empty bucket is an error", objects: map[string][]byte{"dummy/": nil}, @@ -242,8 +262,9 @@ func (suite *S3MetadataTestSuite) TestGetS3DataFromMetadataClient() { // matching the artifact that was attested. func (suite *S3MetadataTestSuite) TestMetadataMatchesDownloadFingerprint() { for _, t := range []struct { - name string - objects map[string][]byte + name string + objects map[string][]byte + excludePaths []string }{ { name: "a single object", @@ -286,6 +307,20 @@ func (suite *S3MetadataTestSuite) TestMetadataMatchesDownloadFingerprint() { "b.txt": []byte(fakeTemplateBody), }, }, + { + // Excluding a root .kosli_ignore makes the two sources agree again: + // content mode never downloads it, so DirSha256 finds no ignore file + // and applies no rules, which is what metadata mode does too. The + // error message for an un-excluded .kosli_ignore says as much, so + // pin it here rather than leaving the claim untested. + name: "an excluded root .kosli_ignore", + objects: map[string][]byte{ + ".kosli_ignore": []byte("notes.txt\n"), + "README.md": []byte(fakeReadmeBody), + "notes.txt": []byte(fakeNotesBody), + }, + excludePaths: []string{".kosli_ignore"}, + }, } { suite.Run(t.name, func() { newClient := func() *FakeS3Client { @@ -297,11 +332,11 @@ func (suite *S3MetadataTestSuite) TestMetadataMatchesDownloadFingerprint() { } downloaded, err := getS3DataFromClient(newClient(), fakeS3TestBucketName, - nil, nil, nil, nil, logger.NewStandardLogger()) + nil, nil, t.excludePaths, nil, logger.NewStandardLogger()) require.NoError(suite.T(), err) fromMetadata, err := getS3DataFromMetadataClient(newClient(), fakeS3TestBucketName, - nil, nil, nil, nil, logger.NewStandardLogger()) + nil, nil, t.excludePaths, nil, logger.NewStandardLogger()) require.NoError(suite.T(), err) require.Equal(suite.T(), downloaded, fromMetadata,