diff --git a/lab/internal/cli/plugin_new.go b/lab/internal/cli/plugin_new.go index 9ab5e43..8ea11de 100644 --- a/lab/internal/cli/plugin_new.go +++ b/lab/internal/cli/plugin_new.go @@ -71,7 +71,11 @@ func runPluginNew(args []string, out, stderr io.Writer) int { fmt.Fprintf(out, " node -p 'require(\"fs\").realpathSync(process.execPath)'\n") default: fmt.Fprintf(out, "Wrote the %s plugin skeleton %s.\nNext, build the server, pin the manifest to it and test it:\n", flags["--kind"], name) - fmt.Fprintf(out, " go -C %s build -o %s .\n", shown, name) + // GOWORK=off and an empty GOFLAGS keep a go.work above the directory + // and the user's GOFLAGS from changing what is built. + for _, line := range scaffold.GoBuildCommand(shown, name) { + fmt.Fprintf(out, " %s\n", line) + } fmt.Fprintf(out, " forgecell plugin pin %s --server %s/%s\n", shown, shown, name) fmt.Fprintf(out, " forgecell plugin test %s/%s\n", shown, scaffold.ManifestFile) } diff --git a/lab/internal/plugin/scaffold/hardening_test.go b/lab/internal/plugin/scaffold/hardening_test.go new file mode 100644 index 0000000..059e1ce --- /dev/null +++ b/lab/internal/plugin/scaffold/hardening_test.go @@ -0,0 +1,457 @@ +package scaffold + +import ( + "bytes" + "encoding/json" + "fmt" + "go/format" + "os" + "os/exec" + "path/filepath" + "runtime" + "slices" + "strings" + "testing" + + "github.com/DanielNetzer/forgecell/lab/internal/extension" + "github.com/DanielNetzer/forgecell/lab/internal/formula" + "github.com/DanielNetzer/forgecell/lab/internal/plugin/contract" +) + +// withAfterTempWrite sets the pin seam for one test. +func withAfterTempWrite(t *testing.T, hook func(root *os.Root, tmp string)) { + t.Helper() + afterTempWrite = hook + t.Cleanup(func() { afterTempWrite = nil }) +} + +func TestPinDigestIsOfTheFinalManifest(t *testing.T) { + _, dir, server := pinFixture(t) + p, err := Pin(dir, server) + if err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(filepath.Join(dir, ManifestFile)) + if err != nil || p.ManifestSHA256 != sha(raw) { + t.Fatalf("pinned %s, plugin.json hashes to %s: %v", p.ManifestSHA256, sha(raw), err) + } + + // PinInterpreted returns the digest of the bytes it renamed too. Pin only + // hashes the interpreter, so a stand-in file serves. + base := tempDir(t) + pdir := filepath.Join(base, "py") + if err := Write(pdir, Options{Kind: "tracker", Lang: "python", Name: "py"}); err != nil { + t.Fatal(err) + } + interp := writeServer(t, base, "python3") + p, err = PinInterpreted(pdir, filepath.Join(pdir, "server.py"), interp) + if err != nil { + t.Fatal(err) + } + raw, err = os.ReadFile(filepath.Join(pdir, ManifestFile)) + if err != nil || p.ManifestSHA256 != sha(raw) { + t.Fatalf("pinned %s, plugin.json hashes to %s: %v", p.ManifestSHA256, sha(raw), err) + } +} + +// TestPinRefusesASwappedTempManifest changes the temporary manifest between +// its write and its read-back through the seam, and wants each change refused +// with plugin.json and the directory left as they were. +func TestPinRefusesASwappedTempManifest(t *testing.T) { + for _, c := range []struct { + name string + swap func(t *testing.T, root *os.Root, path string) + want string + }{ + {"replaced by an identical file", func(t *testing.T, root *os.Root, path string) { + raw := readFile(t, path) + // Hold the original open so its inode stays allocated: ext4 reuses a + // freed inode number at once, which would give the replacement the + // same identity. + held, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { held.Close() }) + removeFile(t, path) + if err := os.WriteFile(path, raw, 0o600); err != nil { + t.Fatal(err) + } + }, "changed before it was read back"}, + {"replaced by a symlink", func(t *testing.T, root *os.Root, path string) { + removeFile(t, path) + if err := os.Symlink(filepath.Join(filepath.Dir(path), TemplateFile), path); err != nil { + t.Fatal(err) + } + }, "changed before it was read back"}, + {"rewritten in place", func(t *testing.T, root *os.Root, path string) { + raw := readFile(t, path) + if err := os.WriteFile(path, bytes.Replace(raw, []byte(`"pin-me"`), []byte(`"pin-it"`), 1), 0o600); err != nil { + t.Fatal(err) + } + }, "changed before it was read back"}, + {"grown past the manifest bound", func(t *testing.T, root *os.Root, path string) { + f, err := os.OpenFile(path, os.O_WRONLY|os.O_APPEND, 0) + if err != nil { + t.Fatal(err) + } + defer f.Close() + if _, err := f.Write(bytes.Repeat([]byte(" "), extension.MaxManifestBytes+1)); err != nil { + t.Fatal(err) + } + }, "exceeds the manifest bound"}, + {"made group-writable", func(t *testing.T, root *os.Root, path string) { + chmod(t, path, 0o620) + }, "group- or world-writable"}, + {"hard-linked", func(t *testing.T, root *os.Root, path string) { + if err := os.Link(path, filepath.Join(filepath.Dir(filepath.Dir(path)), "alias")); err != nil { + t.Fatal(err) + } + }, "more than one hard link"}, + } { + t.Run(c.name, func(t *testing.T) { + base, dir, server := pinFixture(t) + if _, err := Pin(dir, server); err != nil { + t.Fatal(err) + } + manifest := filepath.Join(dir, ManifestFile) + before := readFile(t, manifest) + files := tree(t, dir) + // A rebuilt server, so the refused pin would have changed plugin.json. + if err := os.WriteFile(server, []byte("#!/bin/true\n"), 0o700); err != nil { + t.Fatal(err) + } + called := false + withAfterTempWrite(t, func(root *os.Root, tmp string) { + called = true + c.swap(t, root, filepath.Join(dir, tmp)) + }) + _, err := Pin(dir, server) + if !called { + t.Fatal("the seam did not run") + } + if err == nil || !strings.Contains(err.Error(), c.want) { + t.Fatalf("got %v, want %q", err, c.want) + } + if strings.Contains(err.Error(), base) { + t.Fatalf("the error names a local path: %v", err) + } + if after := readFile(t, manifest); !bytes.Equal(after, before) { + t.Fatal("a refused pin changed plugin.json") + } + if after := tree(t, dir); !slices.Equal(after, files) { + t.Fatalf("the refusal left %v, was %v", after, files) + } + }) + } +} + +func TestWriteRefusesAGroupOrWorldWritableAdoptedDirectory(t *testing.T) { + for _, mode := range []os.FileMode{0o770, 0o707, 0o720, 0o702} { + t.Run(mode.String(), func(t *testing.T) { + base := tempDir(t) + dir := filepath.Join(base, "open") + mkdir(t, dir) + chmod(t, dir, mode) + before := tree(t, base) + err := Write(dir, Options{Kind: "tracker", Lang: "go", Name: "open"}) + if err == nil || !strings.Contains(err.Error(), "group- or world-writable") { + t.Fatalf("got %v", err) + } + if strings.Contains(err.Error(), base) { + t.Fatalf("the error names a local path: %v", err) + } + if after := tree(t, base); !slices.Equal(after, before) { + t.Fatalf("the refusal left %v, was %v", after, before) + } + if info, err := os.Lstat(dir); err != nil || info.Mode().Perm() != mode { + t.Fatalf("the directory is now %v, %v", info.Mode(), err) + } + }) + } + for _, mode := range []os.FileMode{0o700, 0o755} { + dir := filepath.Join(tempDir(t), "private") + mkdir(t, dir) + chmod(t, dir, mode) + if err := Write(dir, Options{Kind: "tracker", Lang: "go", Name: "private"}); err != nil { + t.Fatalf("a %v directory was refused: %v", mode, err) + } + } +} + +func TestGoBuildCommandIgnoresGoWorkAndGoFlags(t *testing.T) { + got := GoBuildCommand("./demo", "demo") + if !slices.Equal(got, []string{`GOWORK=off GOFLAGS= \`, "go -C ./demo build -o demo ."}) { + t.Fatalf("build command %q", got) + } + dir := filepath.Join(tempDir(t), "built") + if err := Write(dir, Options{Kind: "forge", Lang: "go", Name: "built"}); err != nil { + t.Fatal(err) + } + readme := string(readFile(t, filepath.Join(dir, "README.md"))) + for _, want := range []string{GoBuildEnv + " go build -o built .", "GOWORK=off", "go.work", "GOFLAGS"} { + if !strings.Contains(readme, want) { + t.Fatalf("README.md lacks %q", want) + } + } +} + +// TestReadmeFormulaSnippetParses parses every skeleton README's Formula +// snippet, for every kind and language, with the real Formula parser. +func TestReadmeFormulaSnippetParses(t *testing.T) { + for _, lang := range append([]string{"go"}, interpLangs...) { + for _, kind := range contract.Kinds { + t.Run(lang+"/"+kind, func(t *testing.T) { + name := "my-" + kind + dir := filepath.Join(tempDir(t), name) + if err := Write(dir, Options{Kind: kind, Lang: lang, Name: name}); err != nil { + t.Fatal(err) + } + snippets, err := formulaSnippets(string(readFile(t, filepath.Join(dir, "README.md")))) + if err != nil || len(snippets) != 1 { + t.Fatalf("%d snippets, %v", len(snippets), err) + } + raw, err := snippetFormula(snippets[0]) + if err != nil { + t.Fatal(err) + } + f, err := formula.Parse(raw) + if err != nil { + t.Fatalf("the snippet does not parse: %v\n%s", err, snippets[0]) + } + if len(f.Plugins) != 1 || f.Plugins[0].Name != name { + t.Fatalf("plugins %+v", f.Plugins) + } + }) + } + } +} + +func TestGoSkeletonDocumentsReasonsAndLimits(t *testing.T) { + for _, kind := range contract.Kinds { + c, _ := contract.Lookup(kind) + dir := filepath.Join(tempDir(t), "doc") + if err := Write(dir, Options{Kind: kind, Lang: "go", Name: "doc"}); err != nil { + t.Fatal(err) + } + mainGo := readFile(t, filepath.Join(dir, "main.go")) + if formatted, err := format.Source(mainGo); err != nil || !bytes.Equal(formatted, mainGo) { + t.Fatalf("%s: main.go is not gofmt-clean: %v", kind, err) + } + for _, tool := range c.Tools { + if want := fmt.Sprintf("{%q, %d, %d},", tool.Name, tool.MaxOutputBytes, tool.TimeoutMS); !bytes.Contains(mainGo, []byte(want)) { + t.Fatalf("%s: main.go lacks the limits %s", kind, want) + } + } + readme := string(readFile(t, filepath.Join(dir, "README.md"))) + for _, want := range []string{"auth-missing", "rate-limited", "network", "`failed`", "fail(reason, message)", "maxOutputBytes", "timeoutMs"} { + if !strings.Contains(readme, want) { + t.Fatalf("%s: README.md lacks %q", kind, want) + } + } + } + guide := readGuide(t) + for _, want := range []string{"GOWORK=off GOFLAGS= go -C ./demo-tracker build", "go.work", "fail(reason, message)", "`authMissing`"} { + if !strings.Contains(guide, want) { + t.Fatalf("the plugin guide lacks %q", want) + } + } +} + +// skeletonHooks is a file added to a built forge skeleton: handlers that +// exercise each reason, the output bound and the timeout. The handler context +// seam records each call's timeout and lets forge_describe end its own context +// and then never return, as a hung handler past its timeout does. +const skeletonHooks = `package main + +import ( + "context" + "strings" + "time" +) + +var lastTimeout time.Duration + +func init() { + var cancelCurrent context.CancelFunc + handlerContext = func(ctx context.Context, d time.Duration) (context.Context, context.CancelFunc) { + lastTimeout = d + c, cancel := context.WithCancel(ctx) + cancelCurrent = cancel + return c, cancel + } + handlers["forge_policy"] = func(context.Context, map[string]any) (any, error) { return nil, fail(authMissing, "no token") } + handlers["forge_find_change"] = func(context.Context, map[string]any) (any, error) { return nil, fail(rateLimited, "slow down") } + handlers["forge_change"] = func(context.Context, map[string]any) (any, error) { return nil, fail(network, "unreachable") } + handlers["forge_checks"] = func(context.Context, map[string]any) (any, error) { return nil, fail("bogus", "odd") } + handlers["forge_comments"] = func(context.Context, map[string]any) (any, error) { + return map[string]any{"timeoutMs": lastTimeout.Milliseconds()}, nil + } + handlers["forge_publish_draft"] = func(context.Context, map[string]any) (any, error) { + return map[string]any{"pad": strings.Repeat("P", %d)}, nil + } + handlers["forge_describe"] = func(context.Context, map[string]any) (any, error) { + cancelCurrent() + select {} + } +} +` + +// argsFor returns arguments that pass the skeleton's top-level check: every +// required member with a value of its schema type. +func argsFor(t *testing.T, tool *contract.Tool) string { + t.Helper() + var schema struct { + Properties map[string]struct { + Type any `json:"type"` + } `json:"properties"` + Required []string `json:"required"` + } + if err := json.Unmarshal(tool.InputSchema(), &schema); err != nil { + t.Fatal(err) + } + args := map[string]any{} + for _, name := range schema.Required { + typ := schema.Properties[name].Type + if list, ok := typ.([]any); ok && len(list) > 0 { + typ = list[0] + } + args[name] = map[any]any{"string": "x", "object": map[string]any{}, "integer": 1, "number": 1, "array": []any{}, "boolean": false}[typ] + } + raw, err := json.Marshal(args) + if err != nil { + t.Fatal(err) + } + return string(raw) +} + +// TestSkeletonWireHardening builds a forge skeleton with skeletonHooks and +// checks the raw-line JSON check, each failure reason, the output bound and the +// timeout on the wire. +func TestSkeletonWireHardening(t *testing.T) { + if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.Skip("plugins run only on Linux and macOS") + } + c, _ := contract.Lookup("forge") + toolByName := map[string]*contract.Tool{} + for i := range c.Tools { + toolByName[c.Tools[i].Name] = &c.Tools[i] + } + draft := toolByName["forge_publish_draft"] + comments := toolByName["forge_comments"] + if draft == nil || comments == nil || toolByName["forge_describe"] == nil { + t.Fatal("the forge contract lacks a tool the test calls") + } + + base := tempDir(t) + dir := filepath.Join(base, "hooks") + if err := Write(dir, Options{Kind: "forge", Lang: "go", Name: "hooks"}); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "hooks.go"), []byte(fmt.Sprintf(skeletonHooks, draft.MaxOutputBytes)), 0o600); err != nil { + t.Fatal(err) + } + server := filepath.Join(base, "server") + buildSkeleton(t, dir, server, base) + + ping := func(id string) string { return `{"jsonrpc":"2.0","id":` + id + `,"method":"ping"}` } + call := func(id, name string) string { + return `{"jsonrpc":"2.0","id":` + id + `,"method":"tools/call","params":{"name":"` + name + `","arguments":` + argsFor(t, toolByName[name]) + `}}` + } + type want struct { + id string // the reply's id, as raw JSON + code int // a JSON-RPC error code, or 0 for a result + reason string // an isError tool result's reason, or "" for a plain result + text string // a substring of the reply + } + lines := []struct { + name, input string + want want + }{ + {"leading U+00A0", " " + ping("1"), want{id: "null", code: -32700}}, + {"leading U+0085", "\u0085" + ping("2"), want{id: "null", code: -32700}}, + {"trailing U+00A0", ping("3") + " ", want{id: "null", code: -32700}}, + {"JSON whitespace around", " \t" + ping("4") + " \t", want{id: "4"}}, + {"auth-missing", call("5", "forge_policy"), want{id: "5", reason: "auth-missing", text: "no token"}}, + {"rate-limited", call("6", "forge_find_change"), want{id: "6", reason: "rate-limited"}}, + {"network", call("7", "forge_change"), want{id: "7", reason: "network"}}, + {"an unknown reason", call("8", "forge_checks"), want{id: "8", reason: "failed"}}, + {"the contract timeout", call("9", "forge_comments"), want{id: "9", text: fmt.Sprintf(`"timeoutMs":%d`, comments.TimeoutMS)}}, + {"over the output bound", call("10", "forge_publish_draft"), want{id: "10", reason: "failed", text: "output bound"}}, + {"timed out", call("11", "forge_describe"), want{id: "11", reason: "failed", text: "timed out"}}, + {"while the hung handler runs", call("12", "forge_comments"), want{id: "12", reason: "failed", text: "still running"}}, + {"ping after the hung handler", ping("13"), want{id: "13"}}, + } + var in strings.Builder + for _, l := range lines { + in.WriteString(l.input + "\n") + } + cmd := exec.Command(server) + cmd.Dir = base + cmd.Env = []string{} + cmd.Stdin = strings.NewReader(in.String()) + var stderr strings.Builder + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil || stderr.Len() != 0 { + t.Fatalf("the server failed: %v\n%s", err, stderr.String()) + } + replies := strings.Split(strings.TrimSuffix(string(out), "\n"), "\n") + if len(replies) != len(lines) { + t.Fatalf("%d replies for %d lines: %q", len(replies), len(lines), replies) + } + for i, l := range lines { + raw := replies[i] + var reply struct { + ID json.RawMessage `json:"id"` + Error *struct { + Code int `json:"code"` + } `json:"error"` + Result *struct { + IsError bool `json:"isError"` + StructuredContent struct { + Reason string `json:"reason"` + } `json:"structuredContent"` + } `json:"result"` + } + if err := json.Unmarshal([]byte(raw), &reply); err != nil || string(reply.ID) != l.want.id { + t.Fatalf("%s: reply %s, %v", l.name, raw, err) + } + switch { + case l.want.code != 0: + if reply.Error == nil || reply.Error.Code != l.want.code { + t.Fatalf("%s: want error %d, got %s", l.name, l.want.code, raw) + } + case l.want.reason != "": + if reply.Result == nil || !reply.Result.IsError || reply.Result.StructuredContent.Reason != l.want.reason { + t.Fatalf("%s: want reason %s, got %s", l.name, l.want.reason, raw) + } + default: + if reply.Error != nil || reply.Result == nil || reply.Result.IsError { + t.Fatalf("%s: want a result, got %s", l.name, raw) + } + } + if !strings.Contains(raw, l.want.text) { + t.Fatalf("%s: reply lacks %q: %s", l.name, l.want.text, raw) + } + if strings.Contains(raw, "PPPP") { + t.Fatalf("%s: part of an over-bound result was sent", l.name) + } + } +} + +func readFile(t *testing.T, path string) []byte { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return raw +} + +func removeFile(t *testing.T, path string) { + t.Helper() + if err := os.Remove(path); err != nil { + t.Fatal(err) + } +} diff --git a/lab/internal/plugin/scaffold/interpreted_test.go b/lab/internal/plugin/scaffold/interpreted_test.go index 7fb8922..4e45a40 100644 --- a/lab/internal/plugin/scaffold/interpreted_test.go +++ b/lab/internal/plugin/scaffold/interpreted_test.go @@ -220,22 +220,22 @@ func TestInterpretedSkeletonFiles(t *testing.T) { // inputs, to its SHA-256 as slice 6a wrote it. func TestGoSkeletonIsByteIdentical(t *testing.T) { want := map[string]string{ - "tracker/README.md": "24f398a470d47b741cdfc4123b4595737feca2ded276864c1a02d7a2a3c79def", + "tracker/README.md": "cb3a7918ca14b000e3c559c849f213f50961f6a8e5b4aeca6c7a112c964281b2", "tracker/fixture.json": "58a87405a1aabccff41579eb664dcd5aeb1a7ffecfdd60b2260bcd768080aefa", "tracker/go.mod": "d40bb225ee099817b4d986475995ee6681eddef32a46c55b12cc606465348649", - "tracker/main.go": "bdb9044eae692bfcd932bdec6dc4863f3051cf16369264e84f7a37a1a4217597", + "tracker/main.go": "527148f0cb6bafa5a0f8c144014fa87e0ce0dfcdcae1045d55c52efb66c7e06d", "tracker/plugin.json.tmpl": "3472c4411f8ca9d36e06fadae0adc91a8d20c9b644df9997c22425c877f15cb6", "tracker/tools.json": "dc5ef221c3aa39cba5ce08400b0a24d1fb8b9947c2dea83d550de2ed71b72b87", - "forge/README.md": "7705eed9442a1e5fedc8166a434c0576538315fde05913800c495d8026f792a5", + "forge/README.md": "0eaeaf986903a63bc91a0b53658064d2e845c4d8301775db5cdd2ba148dd8060", "forge/fixture.json": "63eab287edf4b427af2fbb9dd75e65d738836826e7b5557234573aaa100353b2", "forge/go.mod": "ecc815e9012ac642bd5cd56fe57b2fd58550ff0abf7e73f30f15fea877836401", - "forge/main.go": "041090aec0de96403bb48897e6aa7385aef42c117e918575dfe5c0736107924b", + "forge/main.go": "cf98cc0eedc99ef246fbdccfb003e1699e2b001ea1547a264ebbbdaaf81febbd", "forge/plugin.json.tmpl": "5efc663e2c445e93942b4db8d2a801dd412971da621dc6ee1c9b354c4ca799ca", "forge/tools.json": "1112ceb61726228c3ffa2011f400df901f1397c131a9bb0856007053eac8dca6", - "reviewer/README.md": "c5c5db093b8c00dd930175a153bd718cd699b7f2404907af4d1e23cfa287b8f9", + "reviewer/README.md": "ce0026d426615d4222279d666819ee7d79eab658b588d797415201e0f35bfd8e", "reviewer/fixture.json": "7240275607175e07e3194b8d504c311b12ed366458de0ce3dee438b9d6be5f0a", "reviewer/go.mod": "fc1ccc378d12de9b88ab6231ca57bed7bd87a736fc7aa65729434095209e80aa", - "reviewer/main.go": "d400690099e095e77cb88ee97b34879d06581f3ea374c1b78bd2767460cfcf52", + "reviewer/main.go": "9ab1f42fdd39d68226bfdc39948f5a29fd9a659e4a13b843efeb9fb7764b4ccd", "reviewer/plugin.json.tmpl": "6731514edecc9daa18bf96a78db37f40c01467c2465592c9d372169120744f6b", "reviewer/tools.json": "ed13a5946ded0e0a054a438a1c9ca48bf8b1554a135ad821734552bd69798288", } diff --git a/lab/internal/plugin/scaffold/pin.go b/lab/internal/plugin/scaffold/pin.go index f754af9..284595a 100644 --- a/lab/internal/plugin/scaffold/pin.go +++ b/lab/internal/plugin/scaffold/pin.go @@ -1,6 +1,7 @@ package scaffold import ( + "bytes" "crypto/rand" "crypto/sha256" "encoding/hex" @@ -45,8 +46,10 @@ type Pinned struct { // be a regular file with its owner exec bit, given by a path that is not itself // a symlink, and must pass the engine's placement rules: owned by the user or // root, not group- or world-writable, one hard link, and no ancestor that -// another user owns or can write without the sticky bit. The manifest must load through plugin.LoadFile before it replaces -// anything. Errors never name a local path. +// another user owns or can write without the sticky bit. The manifest is read +// back through the directory handle, hashed and decoded before it replaces +// anything, and the digest returned is of those bytes. Errors never name a +// local path. func Pin(dir, server string) (Pinned, error) { if err := checkServerPath(server); err != nil { return Pinned{}, err @@ -338,16 +341,23 @@ func readTemplate(root *os.Root) (string, error) { return string(raw), nil } -// replaceManifest writes the manifest to a fresh temporary file in dir, loads -// it through plugin.LoadFile and only then renames it over plugin.json. A -// failure removes the temporary file and leaves plugin.json as it was. +// afterTempWrite runs between writing the temporary manifest and reading it +// back; a seam for tests that swap the file there. nil in normal use. +var afterTempWrite func(root *os.Root, tmp string) + +// replaceManifest writes the manifest to a fresh temporary file in dir, reads +// it back through the root handle, hashes and decodes those bytes, and only +// then renames it over plugin.json. The file read back must be the one this +// call wrote, hold exactly the manifest and pass the engine's placement rules, +// so the digest returned describes the file renamed. A failure removes the +// temporary file and leaves plugin.json as it was. func replaceManifest(root *os.Root, dirAbs string, manifest []byte) (Pinned, error) { var suffix [8]byte if _, err := rand.Read(suffix[:]); err != nil { return Pinned{}, errors.New("no random name for the temporary manifest") } tmp := ".plugin.json." + hex.EncodeToString(suffix[:]) + ".tmp" - made, err := writeExclusive(root, tmp, manifest) + made, written, err := writeExclusiveStat(root, tmp, manifest) done := false defer func() { if made && !done { @@ -357,13 +367,70 @@ func replaceManifest(root *os.Root, dirAbs string, manifest []byte) (Pinned, err if err != nil { return Pinned{}, errors.New("writing the manifest failed") } - m, digest, err := plugin.LoadFile(filepath.Join(dirAbs, tmp)) + if afterTempWrite != nil { + afterTempWrite(root, tmp) + } + f, err := openWritten(root, tmp, written) + if err != nil { + return Pinned{}, err + } + defer f.Close() + raw, err := io.ReadAll(io.LimitReader(f, extension.MaxManifestBytes+1)) + switch { + case err != nil: + return Pinned{}, errors.New("the pinned manifest does not load: " + string(mcp.NotInstalled)) + case len(raw) > extension.MaxManifestBytes: + return Pinned{}, errors.New("the pinned manifest exceeds the manifest bound") + case !bytes.Equal(raw, manifest): + return Pinned{}, errors.New("the temporary manifest changed before it was read back") + } + if pinned.CheckPlacement(filepath.Join(dirAbs, tmp)) != nil { + return Pinned{}, errors.New("the pinned manifest does not load: " + string(mcp.NotInstalled)) + } + sum := sha256.Sum256(raw) + m, err := plugin.Decode(raw) if err != nil { - return Pinned{}, errors.New("the pinned manifest does not load: " + string(mcp.ReasonOf(err))) + return Pinned{}, errors.New("the pinned manifest does not load: " + string(mcp.SchemaInvalid)) } if err := root.Rename(tmp, ManifestFile); err != nil { return Pinned{}, errors.New("replacing plugin.json failed") } done = true - return Pinned{Name: m.Name, ManifestSHA256: digest}, nil + if now, err := root.Lstat(ManifestFile); err != nil || !os.SameFile(now, written) { + return Pinned{}, errors.New("plugin.json changed while it was replaced") + } + return Pinned{Name: m.Name, ManifestSHA256: hex.EncodeToString(sum[:])}, nil +} + +// openWritten opens the temporary manifest through the root handle without +// following a symlink and proves it is still the file written: the same file, +// a regular file of one hard link that only its owner, the user, can write. +func openWritten(root *os.Root, tmp string, written fs.FileInfo) (*os.File, error) { + changed := errors.New("the temporary manifest changed before it was read back") + info, err := root.Lstat(tmp) + if err != nil || info.Mode()&fs.ModeSymlink != 0 || !os.SameFile(info, written) { + return nil, changed + } + f, err := root.OpenFile(tmp, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) + if err != nil { + return nil, changed + } + opened, err := f.Stat() + if err != nil || !os.SameFile(opened, written) || !opened.Mode().IsRegular() || !ownedByMe(opened) { + f.Close() + return nil, changed + } + if opened.Mode().Perm()&0o022 != 0 { + f.Close() + return nil, errors.New("the temporary manifest is group- or world-writable") + } + if n, ok := pinned.LinkCount(opened); ok && n > 1 { + f.Close() + return nil, errors.New("the temporary manifest has more than one hard link") + } + if opened.Size() > extension.MaxManifestBytes { + f.Close() + return nil, errors.New("the pinned manifest exceeds the manifest bound") + } + return f, nil } diff --git a/lab/internal/plugin/scaffold/scaffold.go b/lab/internal/plugin/scaffold/scaffold.go index 2a44f07..264662c 100644 --- a/lab/internal/plugin/scaffold/scaffold.go +++ b/lab/internal/plugin/scaffold/scaffold.go @@ -46,7 +46,8 @@ const ( var templates embed.FS // Write creates a skeleton in dir. dir must not exist yet (its parent must), or -// be an empty directory the user owns; it is never a symlink. Files are created +// be an empty directory the user owns that is not group- or world-writable; it +// is never a symlink. Files are created // exclusively, never overwritten, and on any failure only the files this call // created are removed. Errors never name a local path. func Write(dir string, opts Options) error { @@ -108,6 +109,7 @@ func render(c *contract.Contract, name string) ([]skeletonFile, error) { "{{SERVER_NAME}}", quote(name), "{{CONTRACT_ID}}", quote(c.ID), "{{PROTOCOL_VERSION}}", quote(mcp.ProtocolVersion), + "{{LIMITS}}", limitsTable(c), )) if err != nil { return nil, err @@ -121,6 +123,7 @@ func render(c *contract.Contract, name string) ([]skeletonFile, error) { "{{KIND}}", c.Kind(), "{{CONTRACT}}", c.ID, "{{ENABLE}}", enableSnippet(c.Kind(), name), + "{{BUILD_ENV}}", GoBuildEnv, )) if err != nil { return nil, err @@ -135,6 +138,28 @@ func render(c *contract.Contract, name string) ([]skeletonFile, error) { }, nil } +// GoBuildEnv is the environment a Go skeleton is built with: no go.work above +// the directory joins it to another module, and no GOFLAGS of the user's (such +// as -mod=vendor) changes what is built. +const GoBuildEnv = "GOWORK=off GOFLAGS=" + +// GoBuildCommand is the command that builds a Go skeleton in dir into name, as +// two shell lines joined by a backslash continuation: GoBuildEnv, then the go +// build itself. dir and name are printed as given. +func GoBuildCommand(dir, name string) []string { + return []string{GoBuildEnv + ` \`, "go -C " + dir + " build -o " + name + " ."} +} + +// limitsTable renders main.go's per-tool limits from the contract: each tool's +// name, maxOutputBytes and timeoutMs, one tool per line. +func limitsTable(c *contract.Contract) string { + lines := make([]string, 0, len(c.Tools)) + for _, t := range c.Tools { + lines = append(lines, fmt.Sprintf("\t{%s, %d, %d},", strconv.Quote(t.Name), t.MaxOutputBytes, t.TimeoutMS)) + } + return strings.Join(lines, "\n") +} + // renderInterpreted builds a python or typescript skeleton. The engine runs a // private copy of the pinned script alone, so the tool table and the describe // fixture are embedded into the script as literals made from the same @@ -414,9 +439,9 @@ func enableSnippet(kind, name string) string { case "tracker": return "intake:\n source: " + name case "forge": - return "forge:\n plugin: " + name + return "forge:\n plugin: " + name + "\n project: " } - return "reviewers:\n - plugin: " + name + return "reviewers:\n - plugin: " + name + "\n autoFix: none" } // writeFiles creates dir (or adopts it when it is an empty directory the user @@ -475,6 +500,11 @@ func writeFiles(abs string, files []skeletonFile) (err error) { if !info.IsDir() || !ownedByMe(info) { return errors.New("the directory must be a directory you own") } + // An adopted directory must pass the placement rules plugin test applies + // later; one this call created is 0700. + if !created && info.Mode().Perm()&0o022 != 0 { + return errors.New("the directory is group- or world-writable; make it writable by its owner only") + } if root, err = parent.OpenRoot(base); err != nil { return errors.New("the directory cannot be opened") } @@ -519,19 +549,30 @@ func checkEmpty(root *os.Root, want fs.FileInfo) error { // or symlink is never written through. made reports whether this call created // the file, so a failure removes it and never a file it did not create. func writeExclusive(root *os.Root, name string, data []byte) (made bool, err error) { + made, _, err = writeExclusiveStat(root, name, data) + return made, err +} + +// writeExclusiveStat is writeExclusive that also returns the identity of the +// file written, taken from the handle it was written through. +func writeExclusiveStat(root *os.Root, name string, data []byte) (made bool, info fs.FileInfo, err error) { f, err := root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, fileMode) if err != nil { - return false, err + return false, nil, err } if _, err := f.Write(data); err != nil { f.Close() - return true, err + return true, nil, err } if err := f.Sync(); err != nil { f.Close() - return true, err + return true, nil, err + } + if info, err = f.Stat(); err != nil { + f.Close() + return true, nil, err } - return true, f.Close() + return true, info, f.Close() } func ownedByMe(info fs.FileInfo) bool { diff --git a/lab/internal/plugin/scaffold/templates/go/README.md.tmpl b/lab/internal/plugin/scaffold/templates/go/README.md.tmpl index ff52f8b..2b008bd 100644 --- a/lab/internal/plugin/scaffold/templates/go/README.md.tmpl +++ b/lab/internal/plugin/scaffold/templates/go/README.md.tmpl @@ -22,18 +22,42 @@ That check is not full schema validation. It does not check bounds (`minLength`, look inside nested objects or arrays. When you implement a tool, check those yourself in its handler before you use an argument. -Replace the tools one at a time in `main.go`. Keep `tools.json` as it is: the engine checks -the advertised schemas against the contract. +Replace the tools one at a time in `main.go`: add a `handler` for each to `handlers`. Keep +`tools.json` as it is: the engine checks the advertised schemas against the contract. + +## Errors, output bounds and timeouts + +A handler that can't answer returns `fail(reason, message)`. The reason is one of: + +| Reason | Constant | Use it when | +|---|---|---| +| `auth-missing` | `authMissing` | A credential the plugin needs is unset, expired or refused (for example HTTP 401 or 403). | +| `rate-limited` | `rateLimited` | The service asks you to slow down (for example HTTP 429). | +| `network` | `network` | The service can't be reached: DNS, connection, TLS or a timeout on the plugin's side. | +| `failed` | `failed` | Anything else, including arguments your handler refuses. | + +`fail` turns any other reason into `failed`, and any error that is not from `fail` is +answered with reason `failed` and a fixed message. Forgecell shows only the reason, so keep +the message short and free of secrets and local paths; the server cuts it to 200 bytes. + +`limits` in `main.go` holds each tool's contract `maxOutputBytes` and `timeoutMs`, copied +when the skeleton was made. The server enforces both itself: a result whose structured +JSON is larger than the tool's `maxOutputBytes` is never sent, and a call that runs past +its `timeoutMs` is answered when the timeout fires. Both fail with reason `failed`. A +handler gets a context that ends at the timeout and must stop when it does: until a +timed-out handler returns, every later call fails. ## Build From this directory, build the server: ```sh -go build -o {{NAME}} . +{{BUILD_ENV}} go build -o {{NAME}} . ``` -The server takes no arguments. +`GOWORK=off` builds this module on its own even when a `go.work` file in a directory above +would join it to other modules, and the empty `GOFLAGS` keeps flags from your environment, +such as `-mod=vendor`, from changing what is built. The server takes no arguments. ## Pin the manifest @@ -70,5 +94,7 @@ Put this directory in your Lab as `plugins/{{NAME}}`, pin it there, and add the ```yaml {{ENABLE}} plugins: - {{NAME}}: {manifest: plugins/{{NAME}}/plugin.json, sha256: "", required: false} + - {name: {{NAME}}, manifest: plugins/{{NAME}}/plugin.json, sha256: ""} ``` + +`plugins:` takes exactly `name`, `manifest` and `sha256`; there is no `required` key. diff --git a/lab/internal/plugin/scaffold/templates/go/main.go.tmpl b/lab/internal/plugin/scaffold/templates/go/main.go.tmpl index 91e4bad..8a053b1 100644 --- a/lab/internal/plugin/scaffold/templates/go/main.go.tmpl +++ b/lab/internal/plugin/scaffold/templates/go/main.go.tmpl @@ -6,7 +6,14 @@ // tools.json is the contract's tool table and fixture.json holds the describe // answer; both are embedded. The describe tool answers the fixture, and every // other tool fails with "not implemented yet". Replace those tools one at a -// time. +// time by adding a handler for each to handlers. A handler names why it failed +// with fail: authMissing, rateLimited, network or failed. +// +// Each call runs under its tool's contract timeout (timeoutMs), and its +// structured result is bounded by the tool's contract maxOutputBytes; both +// come from limits. A larger result is never sent, and a call that times out +// is answered with reason failed. A handler that ignores its context and +// outlives its timeout makes every later call fail until it returns. // // Every tool first runs accepts on its arguments, which checks exactly three // things: each top-level required member is present, no unknown top-level @@ -24,6 +31,7 @@ package main import ( "bufio" "bytes" + "context" _ "embed" "encoding/json" "errors" @@ -31,6 +39,7 @@ import ( "os" "strconv" "strings" + "time" "unicode/utf8" ) @@ -53,6 +62,48 @@ const ( codeInvalidParams = -32602 ) +// The reasons a failed tool call names: authMissing when a credential the +// plugin needs is unset, expired or refused; rateLimited when the service asks +// the plugin to slow down; network when the service cannot be reached; and +// failed for anything else. The engine reads any other reason as failed. +const ( + authMissing = "auth-missing" + rateLimited = "rate-limited" + network = "network" + failed = "failed" +) + +// maxFailureBytes bounds the message a failed call sends back. +const maxFailureBytes = 200 + +// limit is one tool's contract bounds: the most bytes of structured JSON one +// result may hold, and how long one call may run, in milliseconds. +type limit struct { + tool string + maxOutputBytes int + timeoutMs int +} + +// limits are copied from the contract when the skeleton was made. Every tool +// in tools.json needs one. +var limits = []limit{ +{{LIMITS}} +} + +// handler answers one tool call whose arguments passed accepts. It returns a +// result that encodes as a JSON object, or an error: return fail(reason, +// message) to name a reason; any other error is answered with reason failed and +// a fixed message. It must stop when ctx ends. +type handler func(ctx context.Context, args map[string]any) (any, error) + +// handlers serve the tools you have implemented, by tool name. A tool without +// one answers its fixture when it is the describe tool, and fails with "not +// implemented yet" otherwise. +var handlers = map[string]handler{} + +// handlerContext derives one call's context from its timeout; a seam for tests. +var handlerContext = context.WithTimeout + //go:embed tools.json var toolsJSON []byte @@ -76,12 +127,17 @@ type tool struct { Access string `json:"access"` InputSchema json.RawMessage `json:"inputSchema"` schema inputSchema + maxOutput int + timeout time.Duration } type server struct { tools []tool byName map[string]int fixtures map[string]json.RawMessage + + // running is closed when the last handler returns; nil when none ran. + running chan struct{} } func main() { @@ -96,7 +152,9 @@ func main() { } } -// load reads the embedded tool table and fixtures. +// load reads the embedded tool table and fixtures, and attaches each tool's +// limits. A tool without limits, or a handler for a tool that is not listed, is +// refused. func load() (*server, error) { var table struct { Tools []tool `json:"tools"` @@ -104,6 +162,13 @@ func load() (*server, error) { if err := json.Unmarshal(toolsJSON, &table); err != nil { return nil, err } + bounds := map[string]limit{} + for _, l := range limits { + if l.maxOutputBytes < 1 || l.timeoutMs < 1 { + return nil, errors.New("a tool's limits are not positive") + } + bounds[l.tool] = l + } s := &server{byName: map[string]int{}} if err := json.Unmarshal(fixtureJSON, &s.fixtures); err != nil { return nil, err @@ -124,9 +189,19 @@ func load() (*server, error) { return nil, err } t.InputSchema = compact.Bytes() + l, ok := bounds[t.Name] + if !ok { + return nil, errors.New("a tool has no limits") + } + t.maxOutput, t.timeout = l.maxOutputBytes, time.Duration(l.timeoutMs)*time.Millisecond s.tools = append(s.tools, t) s.byName[t.Name] = i } + for name := range handlers { + if _, ok := s.byName[name]; !ok { + return nil, errors.New("a handler serves a tool that is not listed") + } + } return s, nil } @@ -198,13 +273,15 @@ func readLine(r *bufio.Reader, limit int) (line []byte, tooLarge bool, err error } } -// handle answers one message; nil means no reply. +// handle answers one message; nil means no reply. The raw line must be valid +// UTF-8 JSON, so only JSON whitespace may surround the object: a line that +// starts with any other space, such as U+00A0 or U+0085, is a parse error. func (s *server) handle(line []byte) map[string]any { trimmed := bytes.TrimSpace(line) if len(trimmed) == 0 { return nil } - if !utf8.Valid(trimmed) || !json.Valid(trimmed) { + if !utf8.Valid(line) || !json.Valid(line) { return errorMessage(nil, codeParseError, "parse error") } if trimmed[0] != '{' || !uniqueKeys(trimmed) { @@ -281,17 +358,92 @@ func (s *server) call(id json.RawMessage, params map[string]json.RawMessage) map dec.UseNumber() var args map[string]any if err := dec.Decode(&args); err != nil || !t.schema.accepts(args) { - return resultMessage(id, failure("the arguments do not match the input schema")) - } - if strings.HasSuffix(t.Name, "_describe") { - if describe, ok := s.fixtures[t.Name]; ok { - return resultMessage(id, map[string]any{ - "content": []any{map[string]any{"type": "text", "text": string(describe)}}, - "structuredContent": describe, - }) + return resultMessage(id, failure(failed, "the arguments do not match the input schema")) + } + return resultMessage(id, s.run(t, args)) +} + +// unimplemented serves a tool that has no handler: the describe tool answers +// its fixture, and any other tool fails with "not implemented yet". +func (s *server) unimplemented(t tool) handler { + return func(context.Context, map[string]any) (any, error) { + if strings.HasSuffix(t.Name, "_describe") { + if describe, ok := s.fixtures[t.Name]; ok { + return describe, nil + } } + return nil, fail(failed, "not implemented yet: "+t.Name) + } +} + +type outcome struct { + result any + err error +} + +// run calls the tool's handler under its timeout and renders the result. The +// result must encode as a JSON object within the tool's output bound, or the +// call fails with reason failed and nothing of it is sent. A handler that +// ignores its context and outlives its timeout keeps the server from starting +// another until it returns: later calls fail meanwhile, so handlers never +// overlap. +func (s *server) run(t tool, args map[string]any) map[string]any { + if s.running != nil { + select { + case <-s.running: + default: + return failure(failed, "an earlier call is still running") + } + } + h := handlers[t.Name] + if h == nil { + h = s.unimplemented(t) + } + ctx, cancel := handlerContext(context.Background(), t.timeout) + defer cancel() + done := make(chan outcome, 1) + running := make(chan struct{}) + s.running = running + // The handler is marked finished before its outcome is sent, so once run + // receives the outcome a later call never finds it still running. + go func() { + var o outcome + defer func() { + if recover() != nil { + o = outcome{err: errors.New("the handler panicked")} + } + close(running) + done <- o + }() + result, err := h(ctx, args) + o = outcome{result, err} + }() + var o outcome + select { + case o = <-done: + case <-ctx.Done(): + } + if ctx.Err() != nil { + return failure(failed, "the call timed out") + } + if o.err != nil { + var e *toolError + if errors.As(o.err, &e) { + return failure(e.reason, e.message) + } + return failure(failed, "the tool failed") + } + structured, err := json.Marshal(o.result) + if _, ok := object(structured); err != nil || !ok { + return failure(failed, "the tool returned no JSON object") + } + if len(structured) > t.maxOutput { + return failure(failed, "the result exceeds the tool's output bound") + } + return map[string]any{ + "content": []any{map[string]any{"type": "text", "text": string(structured)}}, + "structuredContent": json.RawMessage(structured), } - return resultMessage(id, failure("not implemented yet: "+t.Name)) } // accepts checks the top-level required members, unknown top-level members @@ -358,12 +510,46 @@ func jsonType(v any) string { return "" } -// failure is an isError tool result with the reason failed and a fixed message. -func failure(message string) map[string]any { +// toolError is a handler failure with a fixed reason and message. +type toolError struct { + reason, message string +} + +func (e *toolError) Error() string { return e.reason + ": " + e.message } + +// fail returns a handler error that names reason: authMissing, rateLimited or +// network; any other reason becomes failed. Keep message short, fixed and free +// of secrets and local paths. Forgecell shows only the reason. +func fail(reason, message string) error { + return &toolError{reason: knownReason(reason), message: message} +} + +// knownReason returns reason when it is one the engine reads, and failed +// otherwise. +func knownReason(reason string) string { + switch reason { + case authMissing, rateLimited, network: + return reason + } + return failed +} + +// failure is an isError tool result with a known reason and a message cut to +// maxFailureBytes. +func failure(reason, message string) map[string]any { + reason = knownReason(reason) + message = strings.ToValidUTF8(message, "") + if len(message) > maxFailureBytes { + n := maxFailureBytes + for n > 0 && !utf8.RuneStart(message[n]) { + n-- + } + message = message[:n] + } return map[string]any{ "isError": true, "content": []any{map[string]any{"type": "text", "text": message}}, - "structuredContent": map[string]any{"reason": "failed", "message": message}, + "structuredContent": map[string]any{"reason": reason, "message": message}, } } diff --git a/lab/internal/plugin/scaffold/templates/python/README.md.tmpl b/lab/internal/plugin/scaffold/templates/python/README.md.tmpl index 7a429ed..a6c4eab 100644 --- a/lab/internal/plugin/scaffold/templates/python/README.md.tmpl +++ b/lab/internal/plugin/scaffold/templates/python/README.md.tmpl @@ -69,5 +69,7 @@ Put this directory in your Lab as `plugins/{{NAME}}`, pin it there, and add the ```yaml {{ENABLE}} plugins: - {{NAME}}: {manifest: plugins/{{NAME}}/plugin.json, sha256: "", required: false} + - {name: {{NAME}}, manifest: plugins/{{NAME}}/plugin.json, sha256: ""} ``` + +`plugins:` takes exactly `name`, `manifest` and `sha256`; there is no `required` key. diff --git a/lab/internal/plugin/scaffold/templates/typescript/README.md.tmpl b/lab/internal/plugin/scaffold/templates/typescript/README.md.tmpl index 672388b..60dfda7 100644 --- a/lab/internal/plugin/scaffold/templates/typescript/README.md.tmpl +++ b/lab/internal/plugin/scaffold/templates/typescript/README.md.tmpl @@ -84,5 +84,7 @@ Put this directory in your Lab as `plugins/{{NAME}}`, pin it there, and add the ```yaml {{ENABLE}} plugins: - {{NAME}}: {manifest: plugins/{{NAME}}/plugin.json, sha256: "", required: false} + - {name: {{NAME}}, manifest: plugins/{{NAME}}/plugin.json, sha256: ""} ``` + +`plugins:` takes exactly `name`, `manifest` and `sha256`; there is no `required` key. diff --git a/lab/plugins/README.md b/lab/plugins/README.md index 1490fcf..ad2ab41 100644 --- a/lab/plugins/README.md +++ b/lab/plugins/README.md @@ -118,6 +118,14 @@ unknown method or an unknown tool) with a JSON-RPC error instead. Forgecell name failures with further reasons, such as `timeout`, `output-too-large`, `schema-invalid`, `contract-mismatch`, `not-installed` and `manifest-changed`. A plugin doesn't send those. +In a Go skeleton, a handler names its reason with `fail(reason, message)` and the +constants `authMissing`, `rateLimited`, `network` and `failed`. `fail` turns any other +reason into `failed`, and an error that is not from `fail` is answered with reason +`failed` and a fixed message. The skeleton also enforces each tool's contract +`maxOutputBytes` and `timeoutMs` itself, from the `limits` table `plugin new` copies from +the contract: a larger result is never sent, and a call past its timeout is answered then. +Both fail with reason `failed`. Until a timed-out handler returns, every later call fails. + ## The manifest `plugin.json` is strict JSON: no unknown or repeated keys. `plugin new` writes a template, @@ -181,9 +189,13 @@ interpreter, accepting that this can affect other admin users of that Homebrew. contract has them, check every argument yourself, and keep every answer within its bounds. -3. **Build it or pick the interpreter.** For Go, `go -C ./demo-tracker build -o demo-tracker .`. - For TypeScript, `node ./demo-tracker/build.mjs` builds `server.js`. For Python, there is - nothing to build: find the interpreter's path as shown above. +3. **Build it or pick the interpreter.** For Go, + `GOWORK=off GOFLAGS= go -C ./demo-tracker build -o demo-tracker .`. `GOWORK=off` builds + the skeleton's module on its own even when a `go.work` file in a directory above would + join it to other modules, and the empty `GOFLAGS` keeps flags from your environment, + such as `-mod=vendor`, from changing what is built. For TypeScript, + `node ./demo-tracker/build.mjs` builds `server.js`. For Python, there is nothing to + build: find the interpreter's path as shown above. 4. **Pin it.**