diff --git a/cmd/mesh-host/file_references_test.go b/cmd/mesh-host/file_references_test.go index 83f12e5..d7047f7 100644 --- a/cmd/mesh-host/file_references_test.go +++ b/cmd/mesh-host/file_references_test.go @@ -59,3 +59,34 @@ func TestADeclarationWithAFileByDigestAppliesFromTheObject(t *testing.T) { t.Errorf("1695/AC1: the file holds %d bytes (%v), want %d", len(got), err, len(body)) } } + +// 1695/AC8: the hand-run `apply ` and `reconcile` have no bus: a reference whose file here verifies is applied +// as it is, any other refuses and writes nothing — never an empty file. +func TestTheHandApplyNeverWritesAReferenceEmpty(t *testing.T) { + unansweringPacman(t) + body := []byte(strings.Repeat("a line of a big file\n", 4000)) + dir := t.TempDir() + target := filepath.Join(dir, "big") + opts := options{state: filepath.Join(dir, "state.json"), out: &strings.Builder{}} + raw := referenceDeclaration(target, body) + d, err := declaration.ParseFileTrusted(raw) + if err != nil { + t.Fatal(err) + } + if err := runApply(context.Background(), opts, d, raw, fromFile); err == nil || !strings.Contains(err.Error(), "digest") { + t.Errorf("1695/AC8: a reference with no file and no bus was not refused: %v", err) + } + if _, err := os.Stat(target); err == nil { + t.Error("1695/AC8: a file was written for a reference that could not be had") + } + if err := os.WriteFile(target, body, 0o644); err != nil { + t.Fatal(err) + } + d, _ = declaration.ParseFileTrusted(raw) + if err := runApply(context.Background(), opts, d, raw, fromFile); err != nil && !strings.Contains(err.Error(), "applied only in part") { + t.Fatalf("1695/AC8: %v", err) + } + if got, _ := os.ReadFile(target); string(got) != string(body) { + t.Errorf("1695/AC8: the file holds %d bytes, want %d", len(got), len(body)) + } +} diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index b1ad74e..a0c4e1b 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -608,6 +608,12 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw return errors.New(olderGenerationWords(*refused)) } + // A file given by digest is had before the plan is drawn from it (novox/hq ADR 0486 §1, issue 1695). This command has + // no link to the bus, so a file already here that verifies is used, and any other is refused: never written empty. + if err := apply.ResolveFileReferences(ctx, d, link.FilesFrom(ctx)); err != nil { + return fmt.Errorf("a file of this declaration could not be had by its digest: %w", err) + } + // Said before anything is done. steps := apply.Plan(d, known, origin) if opts.json && opts.dryRun { @@ -2070,13 +2076,6 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto return link.Report{Refused: err.Error()} } order := link.Order{Epoch: declared.Epoch, Sequence: declared.Sequence} - // **A file given by digest is fetched and verified before anything of the declaration is applied** (novox/hq ADR - // 0486 §1, issue 1695): one object missing or differing refuses the declaration whole. The signed bytes are what is - // kept; only what is applied has the content in place of the reference. - if err := apply.ResolveFileReferences(ctx, declared, link.FilesFrom(ctx)); err != nil { - return link.Report{Declared: digestOf(raw), Order: order, - Refused: "a file of this declaration could not be had by its digest: " + err.Error()} - } // **Older than what this node applied is refused, before anything is read from the machine** // (novox/hq to-be 45 §6, ADR 0227 rule 2): a controller that lost its lease and goes on sending. @@ -2121,6 +2120,14 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto known.Mode = apply.ModeOf(declared) } + // **A file given by digest is fetched and verified before anything of the declaration is applied** (novox/hq ADR + // 0486 §1, issue 1695): one object missing or differing refuses the declaration whole. The signed bytes are what is + // kept; only what is applied has the content in place of the reference. + if err := apply.ResolveFileReferences(ctx, declared, link.FilesFrom(ctx)); err != nil { + return link.Report{Declared: digestOf(raw), Order: order, + Refused: "a file of this declaration could not be had by its digest: " + err.Error()} + } + built, err := system.For(builtFor) if err != nil { return link.Report{Refused: err.Error()} diff --git a/internal/apply/apply.go b/internal/apply/apply.go index f723022..f55f20e 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1613,6 +1613,12 @@ func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepF return applyInto(r, previous) } out := begin(r) + // A file given by its digest is content to be had, not content: written as it stands it would be an empty file + // (novox/hq ADR 0486 §1, issue 1695). ResolveFileReferences has put its content in before any apply. + if r.IsReference() { + return out, fmt.Errorf("%s is given by digest %s and was not resolved to its content before the apply; "+ + "it is not written empty", r.Path, r.Digest) + } // What actually goes on disk. For a sealed file the mesh never had this, and neither did // whatever carried the declaration here. diff --git a/internal/apply/references_test.go b/internal/apply/references_test.go index 8effda2..e02ffac 100644 --- a/internal/apply/references_test.go +++ b/internal/apply/references_test.go @@ -9,6 +9,7 @@ import ( "testing" "git.novox.be/novox/mesh-sdk/go/declaration" + "github.com/novox/mesh-host/internal/store" ) func referenceTo(t *testing.T, path string, body []byte) *declaration.File { @@ -117,3 +118,15 @@ func TestBinaryContentAndOneFetchPerDigest(t *testing.T) { t.Errorf("1695/AC5: bytes %q asked %v", d.Resources[0].(*declaration.File).Bytes[:4], asked) } } + +// 1695/AC9: an unresolved reference reaching the apply is refused, never written as an empty file. +func TestAnUnresolvedReferenceIsNeverWrittenEmpty(t *testing.T) { + path := filepath.Join(t.TempDir(), "big") + f := referenceTo(t, path, []byte(strings.Repeat("q", 70000))) + if _, err := applyFile(f, store.Applied{}, nil, nil); err == nil || !strings.Contains(err.Error(), "not written empty") { + t.Errorf("1695/AC9: %v", err) + } + if _, err := os.Stat(path); err == nil { + t.Error("1695/AC9: a file was written") + } +} diff --git a/internal/link/files.go b/internal/link/files.go index 5c52f9d..924db49 100644 --- a/internal/link/files.go +++ b/internal/link/files.go @@ -2,8 +2,11 @@ package link import ( "context" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "errors" "fmt" - "io" "time" "git.novox.be/novox/mesh-sdk/go/declaration" @@ -37,8 +40,31 @@ func FilesFrom(ctx context.Context) func(context.Context, string, int64) ([]byte return nil } -// FileObject reads one object by name from the declaration-files bucket. **Only binds**: the bucket is the controller's -// to make, so one that is not there is said as what it is. Nothing waits longer than ctx allows. +// objectInfo is the part of an object store's metadata record this read needs (nats.go's ObjectInfo, as the store writes it). +type objectInfo struct { + Name string `json:"name"` + NUID string `json:"nuid"` + Size uint64 `json:"size"` + Digest string `json:"digest"` + Deleted bool `json:"deleted"` +} + +// FileObject reads one object by name from the declaration-files bucket, without listing the bucket or looking at +// anything but the object it names (novox/hq ADR 0486 §2: a node-engine's account may read by name and may neither +// list nor write). +// +// **The bus subjects this uses, and no others** (the grant of issue 1697 is exactly these), for bucket B = +// declaration-files, stream S = OBJ_B, name N and its base64 (URL alphabet) form E: +// +// $JS.API.DIRECT.GET.S.$O.B.M.E the object's metadata: the last message of one subject, read directly +// $JS.API.CONSUMER.CREATE.S.> an ordered consumer on the object's chunks (the client names it; the filter is +// the chunk subject, so the grant may end the subject in the filter) +// $O.B.C. the chunks, delivered to the consumer's inbox (the node's own _INBOX.node..>) +// $JS.ACK / flow control none: an ordered consumer takes no acks; its flow control and heartbeats are +// published to the inbox it delivers to +// +// It does not call js.ObjectStore, which asks for the stream's info (a list of every object's name), nor store.Get, which +// reads the metadata with a message get a grant cannot keep from reading by sequence. func (b OverNATS) FileObject(ctx context.Context, name string, limit int64) ([]byte, error) { js := b.JS wait := 10 * time.Second @@ -54,22 +80,49 @@ func (b OverNATS) FileObject(ctx context.Context, name string, limit int64) ([]b } js = bounded } - store, err := js.ObjectStore(declaration.FileBucket) + stream := "OBJ_" + declaration.FileBucket + meta, err := js.GetLastMsg(stream, "$O."+declaration.FileBucket+".M."+base64.URLEncoding.EncodeToString([]byte(name)), + nats.DirectGet(), nats.Context(ctx)) if err != nil { - return nil, fmt.Errorf("the object bucket %s: %w", declaration.FileBucket, err) + if errors.Is(err, nats.ErrMsgNotFound) { + return nil, fmt.Errorf("no object %s in the bucket %s", name, declaration.FileBucket) + } + return nil, fmt.Errorf("the metadata of object %s: %w", name, err) } - res, err := store.Get(name, nats.Context(ctx)) - if err != nil { - return nil, err + var info objectInfo + if err := json.Unmarshal(meta.Data, &info); err != nil { + return nil, fmt.Errorf("the metadata of object %s is not readable: %w", name, err) } - defer res.Close() - body, err := io.ReadAll(io.LimitReader(res, limit+1)) - if err != nil { - return nil, err + if info.Deleted { + return nil, fmt.Errorf("the object %s was deleted from the bucket %s", name, declaration.FileBucket) } - if int64(len(body)) > limit { + if info.Size > uint64(limit) { return nil, fmt.Errorf("the object is larger than the %d bytes the declaration names", limit) } + if info.Size == 0 { + return []byte{}, nil + } + sub, err := js.SubscribeSync("$O."+declaration.FileBucket+".C."+info.NUID, + nats.OrderedConsumer(), nats.BindStream(stream)) + if err != nil { + return nil, fmt.Errorf("reading the chunks of object %s: %w", name, err) + } + defer sub.Unsubscribe() + body := make([]byte, 0, info.Size) + for uint64(len(body)) < info.Size { + msg, err := sub.NextMsgWithContext(ctx) + if err != nil { + return nil, fmt.Errorf("reading the chunks of object %s (%d of %d bytes): %w", name, len(body), info.Size, err) + } + if uint64(len(body))+uint64(len(msg.Data)) > info.Size { + return nil, fmt.Errorf("the chunks of object %s are more than the %d bytes its metadata says", name, info.Size) + } + body = append(body, msg.Data...) + } + sum := sha256.Sum256(body) + if want := "SHA-256=" + base64.URLEncoding.EncodeToString(sum[:]); info.Digest != "" && info.Digest != want { + return nil, fmt.Errorf("the chunks of object %s do not hash to the digest of its metadata", name) + } return body, nil } diff --git a/internal/profile/detectors.go b/internal/profile/detectors.go index 2e9d6dc..7f400bd 100644 --- a/internal/profile/detectors.go +++ b/internal/profile/detectors.go @@ -5,6 +5,8 @@ import ( "fmt" "os" "strings" + + "git.novox.be/novox/mesh-sdk/go/declaration" ) // Named capabilities. Constants rather than strings at the call site, because a capability @@ -32,7 +34,7 @@ const ( // CapFileReferences says this node-engine reads a declaration's file given as a digest and size, fetches it from the // bus's object bucket and checks it before writing (novox/hq ADR 0486 §5, issue 1695). A fact about the program, as // CapContainerGroups: the controller sends references only to a node that reports it. - CapFileReferences = "file-references" + CapFileReferences = declaration.FileReferencesCapability // The network manager this machine runs, one capability per dialect (novox/hq ADR 0161): the // uplink seat's holder declares its own, so the holder for a manager the machine does not run diff --git a/merge-check.sh b/merge-check.sh index dabdee8..14c2060 100755 --- a/merge-check.sh +++ b/merge-check.sh @@ -52,8 +52,8 @@ if [ -z "${MESH_TEST_NATS:-}" ]; then fi if command -v gcc >/dev/null 2>&1; then - CGO_ENABLED=1 go test -race -count=1 ./... + CGO_ENABLED=1 go test -tags integration -race -count=1 ./... else echo "NOT RACE-CHECKED: the toolchain holds no C compiler; the suite runs without the race detector" - CGO_ENABLED=0 go test -count=1 ./... + CGO_ENABLED=0 go test -tags integration -count=1 ./... fi