feat: fingerprint S3 buckets from a virtual tree by mbevc1 · Pull Request #1178 · kosli-dev/cli

mbevc1 · 2026-09-14T17:07:31Z

Record the decision to stop laying bucket objects out by key on the
operator's filesystem. Objects download to anonymous temp files, are
hashed and deleted, and digest.VirtualDirSha256 computes the fingerprint
from (key, sha256) pairs. Captures the compatibility contract, the
shared key rule for content and metadata mode, the rejected CRC64
alternative, and the delivery slices.

Related #1167

Checklist

Docs are autogenerated from CLI help — make sure the help text reflects your changes
Helm chart (charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR
Terraform provider and related changes (terraform-provider-kosli, terraform-aws-evidence-reporter, terraform-aws-kosli-reporter) updated, if needed

Record the decision to stop laying bucket objects out by key on the operator's filesystem. Objects download to anonymous temp files, are hashed and deleted, and digest.VirtualDirSha256 computes the fingerprint from (key, sha256) pairs. Captures the compatibility contract, the shared key rule for content and metadata mode, the rejected CRC64 alternative, and the delivery slices.

path.Clean folds a/../b onto b silently, so the rejection has to run on the raw key. Also spell out that the rule exists for fingerprint compatibility, not safety: DirSha256 can never produce a tree holding ".", ".." or an empty name.

Name the three properties the virtual-tree change must hold, snapshot equals attestation, snapshot equals its own history, and no object is dropped silently, together with the test that holds each one.

…a filesystem VirtualDirSha256 reproduces DirSha256 from (path, sha256) pairs alone: it builds the tree, walks it in filepath.WalkDir order and hashes each entry's base name plus each file's content digest. SingleVirtualFile mirrors containsSingleFile so a one-file tree can take the FileSha256 branch. Every equivalence test materialises the tree on disk and requires DirSha256 to agree, so the two cannot drift silently. No callers yet. This is the first slice of moving kosli snapshot s3 off key-named local files (see docs/adr/20260911-s3-fingerprint-from-virtual-tree.md).

…colliding key virtualPathForS3Key turns an object key into the path it occupies in the virtual tree that will be fingerprinted. A ".." segment is rejected on the raw key, before path.Clean can fold it onto a sibling; a leading slash, "." segments and doubled slashes fold exactly as filepath.Join did, so existing fingerprints are unchanged. Nothing is created under the path, so reserved names, colons, backslashes and overlong components are ordinary names and there is no per-OS branch. virtualPathsForS3Keys validates a whole key set at once and reports every problem together: rejected keys, keys folding onto one path, and an object whose path is also a directory holding other objects, capped at ten. No callers yet; content mode switches to it in a later slice.

VirtualDirSha256 takes the rules of the tree's root .kosli_ignore and excludes what DirSha256 would exclude on disk. Rather than reimplement what the globs appear to mean, virtualFS reproduces filepathx.Glob, filepath.Glob and filepath.Walk step for step over the virtual tree, so their quirks come out identical: a literal "**/x" never matches at the root because the pieces concatenate to a double slash, "**/*.log" does, and excluding "logs/*" leaves an empty directory whose name is still hashed. Exclusion therefore happens inside the tree walk, not by filtering the file list. The root ignore file is never excluded by its own rules, as on disk. ParseIgnoreRules is extracted from excludePathsFromFile so callers that hold the file's bytes get the same reading DirSha256 gives the file. Every rule set in the equivalence test is materialised on disk and fingerprinted with DirSha256, and the virtual digest must match; rows that should change the digest also assert that they do.

…s local paths Each object now downloads to an anonymous temp file that is hashed and removed; the fingerprint comes from digest.VirtualDirSha256 over the (key, sha256) pairs, which reproduces what DirSha256 gave the same tree on disk. No key ever names a file, so traversal, overwrites, reserved names, case folding and component length limits stop being properties of this code, and the fingerprint is the same on every operating system. A root .kosli_ignore is downloaded first and its rules applied; objects the rules exclude are not downloaded at all. digest.FilesNeedingContent decides which files need a digest, and VirtualDirSha256 refuses a tree that needs a digest it was not given, so a skipped download can never leak into a fingerprint. Keys that cannot form a directory tree (a ".." segment, two keys folding onto one path, an object that is also a prefix) fail the snapshot and name every key involved. Fingerprints recorded against main are pinned before the switch and unchanged after it; a bucket built from a directory fingerprints as DirSha256 fingerprints the directory. localPathForS3Key, the O_EXCL and ENOTDIR handling and containsSingleFile are gone with their tests. The cmd/kosli TestSnapshotS3 suite needs the local Kosli server and is left to CI.

Drop references that only made sense during development: the throwaway experiment and the gitignored TODO.md. Name the tests that hold each guarantee, describe the .kosli_ignore resolution as the filepathx simulation that was actually built, mark parallel downloads as delivered separately in #1167, and correct the claim that the codebase gets smaller: the platform-dependent code is gone, but the faithful glob simulation costs more lines than the fenced layout it replaces.

claude · 2026-09-14T17:08:16Z

Claude finished @mbevc1's task in 5m 51s —— View job

Review — push `12a90d41`

Static analysis

Check	Result
`go build ./...`	clean
`go vet ./...`	clean
`go mod tidy` + `git diff go.mod go.sum`	no diff
`go test ./internal/digest/... ./internal/aws/...`	not run — blocked by the sandbox in this job. Please confirm locally / in CI.

The `../tree/x` fix (`12a90d41`)

Verified against filepath.Join and path/filepath.globWithLimit. escapesVirtualRoot tracks depth over the raw rule's segments, which is exactly how Join resolves .. lexically, so:

../tree/app.js → .. at depth 0 → skipped, matching the miss on disk ✅
*/../app.js → depth 1 → 0 → 1, never negative → still applied, matching Join's fold to root/app.js ✅
the malformed-pattern check stays ahead of the skip, so ../tree/a[ still errors on both sides ✅

I also re-walked the mirror against the real sources this round — filepathx.Glob/Globs.Expand (which uses filepath.Walk, reporting the root as given and descendants via Join, exactly as virtualFS.walk does), globWithLimit, cleanGlobPath, glob, and hasMeta. The reproduction is faithful line for line, including the "**/x" double-slash quirk.

One finding

Low / informational — a rule that leaves the root and globs back into it (inline, internal/digest/virtualglob.go:51). ../*/x reliably excludes on disk, because filepath.Glob("/tmp/*/x") expands * over the real /tmp and always re-hits the temp root whatever MkdirTemp named it, while escapesVirtualRoot skips it. Not a regression — the old cleaned-pattern check skipped it too — and it fails safe (disk excludes, virtual keeps), so it is compliance noise rather than a hiding bypass. Suggested a comment sentence rather than code, on the same grounds you gave for the ../**/a[ residue.

Nothing else new. I re-checked, and found no issue with:

the contentSha256 / needed / files[i].Sha256 fill-in loop — indices stay aligned with objects, and both FilesNeedingContent and VirtualDirSha256 derive their exclusion set from paths only, so the empty-digest entries can never be reached
single-object parity with the old containsSingleFile → FileSha256 branch, including when the lone object is .kosli_ignore, and when a filter or an ignore rule reduces the set
newest.IsZero() — no false positive, since a Unix-epoch LastModified is not the zero time.Time
walkIncluded versus calculateDirContentSha256's fs.SkipDir / protected-path branches, and protectedVirtualPath's new directory check against ignoreFilePathInTree
validateVirtualPath's depth bound — >= globSeparatorsLimit separators means at most globSeparatorsLimit segments, which is what the message says
ParseIgnoreRules inheriting bufio.Scanner's 64 KB line limit, which excludePathsFromFile already had, so parity holds
temp-file lifecycle — each download closes and removes its own file, so peak disk really is one object as the ADR claims

Worth calling out

The equivalence harness is the strongest part of this change. TestMatchesDirSha256 materialising the tree and requiring VirtualDirSha256 == DirSha256 for ~30 rule shapes, with hasEffect pinning that the rules actually did something, is what makes a step-for-step mirror of three stdlib functions reviewable at all — the nonexistent/**/a[ and **.log rows in particular encode quirks nobody would rediscover. The listing-fault handling that landed this round (022f3333, 5b4c7a7a) also consistently chose an error over a silent drop, and said so in the message, which keeps the ADR's "no object is lost silently" guarantee honest.
· branch 20260914_s3_virtual_tree

…iew findings filepath.Glob rejects a malformed pattern before it looks at the filesystem, so a rule such as "nonexistent/a[" fails DirSha256 even though nothing could match it. The virtual mirror only surfaced the error from the per-name match, which is never reached when the directory is missing, so the same rule silently excluded nothing. The mirror now validates the pattern up front as Glob does; the parity test covers rules under missing directories and behind a double star. Also from review: the ignore-file closure no longer assigns to the function's named result, and the results are plain now; a bad rule is reported the same way whichever call reaches it, naming the bucket's .kosli_ignore only when the error is a bad pattern; the multi-problem message counts problems rather than keys, since a collision names several; the redundant ignoreFileName alias is gone; and the ADR names s3KeyProblemsError instead of a function that does not exist, and records that the ignore file is now matched by exact key.

…alFile The single-object shortcut open-coded the decision SingleVirtualFile documents and tests, leaving the exported function without a caller. The manifest is now built as paths first, so the shortcut branches on SingleVirtualFile and names the artifact with VirtualFile.Name, and the digests are filled in by index afterwards. FilesNeedingContent takes the manifest itself rather than a parallel slice of paths, which removes one per-object allocation from the run. Also from review: the single-problem advice reads "exclude the affected keys", since a collision names more than one, and the lone-collision message is pinned.

#1175 moved the S3 key containment rule into utils.LocalRelativePath and had localPathForS3Key and unusableS3KeyError call it. This branch deletes those functions, since object keys no longer become local paths, so its versions of internal/aws/aws.go and aws_test.go are kept and the test of localPathForS3Key's sentinel goes with the function. The Azure fix and utils.LocalRelativePath themselves merge unchanged.

…the tree filepath.Glob validates the whole pattern before it looks at the filesystem, so a malformed rule such as "../a[" fails DirSha256 even though it names a path outside the directory. The virtual mirror skipped rules that resolve outside the tree before validating them, so the same rule was silently ignored. The joined pattern is now validated first; path.Clean cannot add or remove glob metacharacters, so the verdict matches the on-disk one. The parity test gains the "../a[" row. Also corrects the maxReportedS3KeyProblems comment, which still counted keys after the message moved to counting problems.

…front The previous commit validated the whole joined pattern, which rejected "nonexistent/**/a[" even though on disk filepathx never evaluates the malformed second piece once the first piece matches nothing. filepathx hands filepath.Glob the first piece unconditionally and later pieces only as earlier ones match, so validating the first piece up front reproduces the on-disk verdict for escaping and non-escaping rules alike. The equivalence row for that rule is green again.

…rules On disk only a file of that name carries rules and is shielded from them; ignoreFilePathInTree skips a directory, so a rule naming it excludes it. The virtual digest protected the name unconditionally and kept the directory and its subtree. The protected path now depends on what the tree holds at that name, and an equivalence test pins the directory case.

The temp file's name is anonymous by design, so a bare hashing error was the one failure in the S3 path an operator could not map back to an object.

… without a key The rewrite dereferenced LastModified on every listed object where the old loop only did so from the second object on, so a listing entry without a timestamp went from a misleading error to a panic. Real S3 always sets it; S3-compatible stores may not. Such an object now stays in the fingerprint and out of the snapshot timestamp, and a listing with no timestamp at all is an error. An entry with no key is an error too, since dropping it would lose an object silently.

Since the collision line was capped, one constant bounds the problems listed and the keys named per collision; the comment named only the first.

The enforcement the comment pointed at lives in the VirtualDirSha256 walk, not in a function of that name.

A key listed twice rendered as a collision of the key with itself and advised excluding the only copy. It is a listing fault, so it is now an error of its own, and the collision report can rely on distinct keys.

…s a pattern The tree walks recurse once per segment over a tree the bucket's writers shape. S3 keys stay far below the bound; it exists for callers that do not get their paths from S3.

…stant The guarantee read as exhaustive but three listing faults and the path depth bound landed after it; the collision cap is now attributed to the constant both caps read.

aws.go now imports path, so the prefix loop in objectMatchesFilter shadowed it.

…a doubled verb

"and 3 more fingerprint as" read as a verb disagreeing with its subject.

The error claims the listing repeated a key, so it must not depend on which filters are in force.

…s the root The escape check ran on the cleaned pattern, so "../tree/x" folded back onto "tree/x" and was applied, where on disk the temp directory's unguessable name keeps a rule that has left the tree outside it. Depth is now tracked over the raw rule's segments, which is how filepath.Join resolves "..", so a rule that dips through a wildcard and returns still matches while one that walks above the root never does.

claude · 2026-09-14T19:48:48Z

+		// that resolves to the root or leaves it cannot match anything there, and
+		// naming the root cannot bring it back, which the cleaned pattern alone
+		// would not show ("../tree/x" cleans to "tree/x").
+		if pattern == virtualRoot || !strings.HasPrefix(pattern, virtualRoot+"/") || escapesVirtualRoot(rule) {


The ../tree/x fix is right, and I traced it against filepath.Join/filepath.Glob — depth counting over the raw rule gives the join's verdict, and */../app.js correctly stays a match because its depth never goes negative.

One residue the new comment doesn't cover, of the same class as the ../**/a[ one two lines up: a rule that leaves the root and globs back into it through a wildcard is applied on disk but skipped here.

For ../*/app.js, with the root at /tmp/bucketContent123:

resolveExcludePaths → filepath.Join(root, "../*/app.js") → /tmp/*/app.js

filepath.Glob expands /tmp/* over the real /tmp, which always contains the root itself, so glob(d, "app.js") yields /tmp/bucketContent123/app.js

that is byte-identical to the path WalkDir emits, so utils.Contains hits and app.js is excluded

Virtually, escapesVirtualRoot("../*/app.js") sees .. at depth 0 → continue → nothing excluded.

Unlike naming the root literally, this doesn't need the attacker to guess anything: * matches the temp directory whatever MkdirTemp called it, so ../*/<name> (and ../../*/*/<name>, …) reliably excludes on disk. Not a regression from 12a90d41 — the old cleaned-pattern check skipped it too — and it fails safe: the direction is disk-excludes / virtual-keeps, so a bucket's ignore rule of this shape makes the snapshot fingerprint differ from the attested directory's rather than letting a file hide. That's compliance noise, not a bypass.

Modelling the temp directory's parent is out of scope for the same reason you gave for ../**/a[, so a sentence rather than code seems right:

Suggested change

if pattern == virtualRoot || !strings.HasPrefix(pattern, virtualRoot+"/") || escapesVirtualRoot(rule) {

// On disk the root is a temp directory with an unguessable name, so a rule

// that resolves to the root or leaves it cannot match anything there, and

// naming the root cannot bring it back, which the cleaned pattern alone

// would not show ("../tree/x" cleans to "tree/x"). A rule that leaves the

// root and globs back into it ("../*/x") does match on disk, where "*" hits

// the temp directory whatever it is called, and is skipped here; as with a

// malformed later piece, modelling the parent would be speculative. It

// fails safe: the digest keeps a file the rule would have dropped.

if pattern == virtualRoot || !strings.HasPrefix(pattern, virtualRoot+"/") || escapesVirtualRoot(rule) {

continue

}

A {name: "a rule that leaves the tree and globs back into it", ignore: "../*/app.js"} row in TestMatchesDirSha256 would document it — note it asserts the virtual side only, since DirSha256 on a t.TempDir() genuinely excludes there, so the two digests would not be equal and the row can't go in that table as-is.

mbevc1 · 2026-09-14T19:50:19Z

This went out of control 🤦

mbevc1 · 2026-09-14T20:04:08Z

Superseded by #1180, which carries the same content rebased onto main as ten commits: the original eight slices plus one commit folding every fix from the review rounds here and one ADR update. Closing this one to keep the history readable.

Generated by Claude Code

mbevc1 added 8 commits September 14, 2026 18:04

docs(adr): state the compatibility and no-silent-loss guarantees

ca5f7cc

Name the three properties the virtual-tree change must hold, snapshot equals attestation, snapshot equals its own history, and no object is dropped silently, together with the test that holds each one.

mbevc1 added the enhancement New feature or request label Sep 14, 2026

github-actions Bot added documentation Improvements or additions to documentation go Pull requests that update go code feat labels Sep 14, 2026