diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 345ab2c..6eadb33 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -173,7 +173,7 @@ func run(ctx context.Context, command string, opts options) error { // can write this file and run this binary can do anything the binary can, so refusing // them an action would buy nothing and would make an action untestable except by // rebuilding the bundle. - d, err := declaration.ParseTrusted(raw) + d, err := declaration.ParseFileTrusted(raw) if err != nil { return err } diff --git a/internal/bundle/bundle.go b/internal/bundle/bundle.go index 856da0f..f9f3c6b 100644 --- a/internal/bundle/bundle.go +++ b/internal/bundle/bundle.go @@ -78,22 +78,5 @@ func Load(system string) (*declaration.Declaration, error) { // ParseTrusted: the bundle arrives with the binary, so it may carry actions the link may // not (novox/hq ADR 0005). The bootstrap needs them — creating the control plane's database // happens before there is any mesh to ask for one. - return declaration.ParseTrusted(stripComments(locks[system])) -} - -// stripComments removes whole-line `//` comments so a bundle can be annotated. -// -// It is JSON on the wire and a pinned, hand-authored artefact here, and a pinned thing nobody -// can annotate is a pinned thing nobody can review. Only whole lines: anything cleverer would -// need to know where strings begin and end, and a parser that half-understands its input is -// worse than one that does not try. -func stripComments(raw []byte) []byte { - var kept []string - for _, line := range strings.Split(string(raw), "\n") { - if strings.HasPrefix(strings.TrimSpace(line), "//") { - continue - } - kept = append(kept, line) - } - return []byte(strings.Join(kept, "\n")) + return declaration.ParseFileTrusted(locks[system]) } diff --git a/internal/bundle/bundle_test.go b/internal/bundle/bundle_test.go index f5f97de..b7fb1ea 100644 --- a/internal/bundle/bundle_test.go +++ b/internal/bundle/bundle_test.go @@ -8,9 +8,6 @@ import ( "github.com/novox/mesh-host/internal/declaration" ) -// declarationParse is the parser Load uses, named here so the test reads as the assertion it is. -func declarationParse(raw []byte) (any, error) { return declaration.Parse(raw) } - func TestADefaultBuildCarriesNothingAndSaysSo(t *testing.T) { // The important one. A host built without a bundle that applied nothing and reported // success would look exactly like a host that raised a first node — and the difference @@ -27,36 +24,20 @@ func TestADefaultBuildCarriesNothingAndSaysSo(t *testing.T) { } } -func TestCommentsAreNotContent(t *testing.T) { - // The placeholder is a comment. If comments counted as content, every default build would - // claim to carry a substrate and then fail to parse it — the right outcome for the wrong - // reason, and a confusing error at the worst moment. - if got := stripComments([]byte("// a\n{\"a\":1}\n // b\n")); strings.Contains(string(got), "//") { - t.Errorf("comments survived stripping: %q", got) - } -} - -func TestOnlyWholeLineCommentsAreStripped(t *testing.T) { - // Anything cleverer would have to know where strings begin and end. A parser that - // half-understands its input is worse than one that does not try — a path containing a - // double slash is ordinary, and losing half of it would be silent. - raw := []byte(`{"path":"https://example.invalid/a"}`) - if got := string(stripComments(raw)); got != string(raw) { - t.Errorf("a slash inside a string was treated as a comment: %q", got) - } -} - func TestABundleWithContentIsParsedByTheSameParserTheLinkWillUse(t *testing.T) { // A bundle that reaches a machine and is then refused by the host carrying it would be a // build-time mistake found at the worst possible moment. + // + // Comment handling itself is asserted where it now lives, in `declaration`. It was here, and + // having it in two places is how it came to be *done* in two places. real := []byte(`// pinned {"declaration":1,"resources":[{"id":"d","type":"directory","path":"/etc/mesh"}]}`) - stripped := stripComments(real) - if strings.Contains(string(stripped), "pinned") { - t.Fatal("the comment survived") + parsed, err := declaration.ParseFileTrusted(real) + if err != nil { + t.Fatalf("an annotated bundle was refused: %v", err) } - if !strings.Contains(string(stripped), "declaration") { - t.Fatal("the declaration did not survive") + if len(parsed.Resources) != 1 { + t.Fatalf("got %d resources", len(parsed.Resources)) } } @@ -70,17 +51,11 @@ func TestWhatValidatesIsWhatIsApplied(t *testing.T) { // return: whatever Load accepts is what gets applied, byte for byte. annotated := []byte("// a comment\n" + `{"declaration":1,"resources":[{"id":"d","type":"directory","path":"/etc/mesh"}]}`) - if _, err := parseFor(annotated); err != nil { + if _, err := declaration.ParseFileTrusted(annotated); err != nil { t.Fatalf("an annotated bundle was refused: %v", err) } } -// parseFor mirrors what Load does to arbitrary bytes, so the test can exercise the path -// without rebuilding the binary with a different embedded file. -func parseFor(raw []byte) (any, error) { - return declarationParse(stripComments(raw)) -} - func TestEverySystemHasABundleAndAndroidsRefuses(t *testing.T) { // The bundle's contents are per system even though its mechanism is not (novox/hq ADR // 0060), so a host must find one built for it — and a host built for a system with no diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 41aad56..3b46d05 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -521,3 +521,36 @@ func vocabulary() string { sort.Strings(names) return strings.Join(names, ", ") } + +// ParseFileTrusted reads a declaration from a file somebody handed this host. +// +// The same as ParseTrusted, and it allows whole-line `//` comments first. A pinned, hand-authored +// artefact that nobody can annotate is one nobody can review — the substrate bundle is mostly +// explanation of why each digest is what it is. +// +// **Only for a file, never for the link.** Over the link the format stays exactly JSON, because +// a wire format with a second thing to strip is a wire format with a second thing to disagree +// about. +// +// It exists because there were two readers for one file: the bundle stripped comments and `apply` +// did not, so the example bundle in this repository could be built into a binary and not applied +// from disk. The failure was `invalid character '/'`, which names the symptom and not the cause. +func ParseFileTrusted(raw []byte) (*Declaration, error) { + return ParseTrusted(stripComments(raw)) +} + +// stripComments removes whole lines beginning with `//`. +// +// Only whole lines: anything cleverer would need to know where strings begin and end, and a +// parser that half-understands its input is worse than one that does not try. A `//` inside a +// value — every image reference has one — is untouched. +func stripComments(raw []byte) []byte { + var kept []string + for _, line := range strings.Split(string(raw), "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "//") { + continue + } + kept = append(kept, line) + } + return []byte(strings.Join(kept, "\n")) +} diff --git a/internal/declaration/declaration_test.go b/internal/declaration/declaration_test.go index 0a647b7..08c7cb7 100644 --- a/internal/declaration/declaration_test.go +++ b/internal/declaration/declaration_test.go @@ -264,3 +264,45 @@ func TestTheVocabularyIsTheSixShapesTheBootstrapNeeds(t *testing.T) { len(speaks), vocabulary()) } } + +func TestADeclarationFromDiskMayBeAnnotated(t *testing.T) { + // The bundle in this repository is mostly explanation of why each digest is what it is, and + // it could be built into a binary and not applied from disk — two readers for one file. The + // failure was `invalid character '/'`, which names the symptom and not the cause. + raw := []byte(`// why this exists +{ + "declaration": 1, + // and why this resource is here + "resources": [ + {"id": "f", "type": "file", "path": "/etc/x", "content": "hello"} + ] +}`) + d, err := ParseFileTrusted(raw) + if err != nil { + t.Fatalf("a file with comments was refused: %v", err) + } + if len(d.Resources) != 1 { + t.Fatalf("got %d resources", len(d.Resources)) + } + // And the wire format is untouched: over the link it is exactly JSON, because a format with + // a second thing to strip is a format with a second thing to disagree about. + if _, err := Parse(raw); err == nil { + t.Fatal("the link accepted a declaration with comments in it") + } +} + +func TestSomethingInsideAValueIsNotAComment(t *testing.T) { + // Every image reference has a `//` in it somewhere near. Only whole lines are dropped. + d, err := ParseFileTrusted([]byte(`{"declaration":1,"resources":[ + {"id":"f","type":"file","path":"/etc/x","content":"see https://example.invalid/ for why"}]}`)) + if err != nil { + t.Fatal(err) + } + file, ok := d.Resources[0].(*File) + if !ok { + t.Fatalf("got %T", d.Resources[0]) + } + if !strings.Contains(file.Content, "https://example.invalid/") { + t.Fatalf("a value was mangled: %q", file.Content) + } +}