pipeline: reject resref collisions when chunking; document erf post-write hashing (#8)
## What Two correctness fixes surfaced in review of the direct-depot HAK artifact work. ### Reject resref+type collisions in `chunksFromManifest` A manifest can name two distinct source paths that collapse to the same `resref+type` (e.g. `creature/foo.tga` and `placeable/foo.tga` — resref is the lowercase basename minus extension). The ERF writer keys resources on `Name:Type`, so the second silently shadowed the first: an asset would vanish from the packed HAK with no error. `chunksFromManifest` now runs `ensureUniqueChunkResources` per chunk and fails the build on a duplicate. This guards **both** build paths — the legacy `--source-manifest` flow and the direct content-addressed flow (`chunksFromSourceManifest` → `chunksFromManifest`). ### Document erf post-write hash coupling `writeResourceData` streams the source into the output while hashing, so size/SHA mismatches are only detected *after* the bytes are written. A non-nil return therefore means the writer holds partial, unverified output and the caller must discard it. Added a comment making that contract explicit; `writeHAKArchive` already honours it (writes to a temp file, removes on any Write error, never renames a bad archive into place). ## Tests - `TestChunksFromManifestRejectsResrefCollision`: collision → error, distinct resrefs → clean. Asserts only error presence/absence — silent asset loss is the contract, not any specific wording. - `go vet ./internal/erf/ ./internal/pipeline/` clean. - `go test ./internal/erf/ ./internal/pipeline/` green. ## Follow-up A new crucible release must be cut after this merges so the guard ships in the binary `sow-assets-manifest` pins. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Reviewed-on: #8 Co-authored-by: vickydotbat <vickydotbat@tutamail.com> Co-committed-by: vickydotbat <vickydotbat@tutamail.com>
This commit is contained in:
@@ -42,3 +42,7 @@ make build # cmd/* -> ./bin
|
||||
make smoke # assert fail-closed contract
|
||||
make image # crucible:<sha>
|
||||
```
|
||||
|
||||
## Tests
|
||||
|
||||
Tests must survive harmless changes to constants, defaults, wording, ordering, fixture data, and internal implementation details. A test that fails merely because a basic value changed is usually a bad test. Only assert exact values when the value is part of a documented public contract, external protocol, compatibility requirement, security rule, migration, or business rule.
|
||||
|
||||
@@ -425,6 +425,11 @@ func writeResourceData(w io.Writer, resource Resource) error {
|
||||
}
|
||||
defer file.Close()
|
||||
|
||||
// The hash is computed while streaming into w, so size/SHA mismatches are
|
||||
// only detected AFTER the (bad) bytes have already been written. A non-nil
|
||||
// return therefore means w holds partially-written, unverified output; the
|
||||
// caller must discard it. writeHAKArchive does this by writing to a temp file
|
||||
// and removing it on any Write error rather than renaming it into place.
|
||||
hash := sha256.New()
|
||||
written, err := io.Copy(io.MultiWriter(w, hash), file)
|
||||
if err != nil {
|
||||
|
||||
@@ -777,7 +777,7 @@ func chunksFromManifest(assets []assetResource, entries []BuildManifestHAK) ([]h
|
||||
}
|
||||
chunkAssets = append(chunkAssets, asset)
|
||||
}
|
||||
chunks = append(chunks, hakChunk{
|
||||
chunk := hakChunk{
|
||||
Config: project.HAKConfig{
|
||||
Name: entry.Group,
|
||||
Priority: entry.Priority,
|
||||
@@ -788,7 +788,14 @@ func chunksFromManifest(assets []assetResource, entries []BuildManifestHAK) ([]h
|
||||
Name: entry.Name,
|
||||
Assets: chunkAssets,
|
||||
Size: erf.ArchiveSize(resourceSlice(chunkAssets)),
|
||||
})
|
||||
}
|
||||
// A manifest can name two distinct source paths that collapse to the same
|
||||
// resref+type (e.g. creature/foo.mdl and placeable/foo.mdl); without this
|
||||
// guard the second would silently shadow the first in the ERF.
|
||||
if err := ensureUniqueChunkResources(chunk); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
chunks = append(chunks, chunk)
|
||||
}
|
||||
return chunks, nil
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"git.westgate.pw/ShadowsOverWestgate/sow-tools/internal/buildinfo"
|
||||
"git.westgate.pw/ShadowsOverWestgate/sow-tools/internal/erf"
|
||||
)
|
||||
|
||||
const validAsset = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
|
||||
@@ -141,6 +142,32 @@ func TestValidateDirectSourceManifestRejectsBuilderMismatch(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Two distinct source paths can collapse to the same resref+type; the chunker
|
||||
// must reject that rather than let one silently shadow the other in the ERF.
|
||||
func TestChunksFromManifestRejectsResrefCollision(t *testing.T) {
|
||||
tga, ok := erf.HAKResourceTypeForExtension("tga")
|
||||
if !ok {
|
||||
t.Fatal("tga is not a hak resource type")
|
||||
}
|
||||
mk := func(rel, resref string) assetResource {
|
||||
r := erf.Resource{Name: resref, Type: tga}
|
||||
return assetResource{Rel: rel, Resource: r, Size: erf.ArchiveSize([]erf.Resource{r})}
|
||||
}
|
||||
entry := BuildManifestHAK{Name: "core_01", Group: "core"}
|
||||
|
||||
collide := []assetResource{mk("creature/foo.tga", "foo"), mk("placeable/foo.tga", "foo")}
|
||||
entry.Assets = []string{"creature/foo.tga", "placeable/foo.tga"}
|
||||
if _, err := chunksFromManifest(collide, []BuildManifestHAK{entry}); err == nil {
|
||||
t.Fatal("expected resref+type collision rejection")
|
||||
}
|
||||
|
||||
distinct := []assetResource{mk("creature/foo.tga", "foo"), mk("placeable/bar.tga", "bar")}
|
||||
entry.Assets = []string{"creature/foo.tga", "placeable/bar.tga"}
|
||||
if _, err := chunksFromManifest(distinct, []BuildManifestHAK{entry}); err != nil {
|
||||
t.Fatalf("distinct resrefs should pack cleanly: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestContentAddressedBlobPath(t *testing.T) {
|
||||
root := "/var/cache/blobs"
|
||||
got, err := contentAddressedBlobPath(root, validAsset)
|
||||
|
||||
Reference in New Issue
Block a user