From c613efe189271bcbf879f15767d712727087ea01 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 02:52:57 +0200 Subject: [PATCH] claude-code: read front matter as YAML and refuse a file reference outside the item A byte-order mark, a flow mapping or a merged key hid a key the line scan looked for, and @path read any file into every session the item reached (hq issue 340 review). --- modules/claude-code/README.md | 5 +- modules/claude-code/cmd/claude-code/config.go | 85 ++++++++++++++----- .../cmd/claude-code/terminal_only_test.go | 50 +++++++++++ modules/claude-code/go.mod | 2 + modules/claude-code/go.sum | 4 + 5 files changed, 126 insertions(+), 20 deletions(-) diff --git a/modules/claude-code/README.md b/modules/claude-code/README.md index f6e4aad8..1c328a08 100644 --- a/modules/claude-code/README.md +++ b/modules/claude-code/README.md @@ -100,7 +100,10 @@ another node. So these tools refuse, and say why and how it is done instead: - `claude_code_mcp_register`: the operator sets tool servers in the `mcp_servers` setting the same way; - a skill, subagent or command that would run something by itself: a shell line the agent runs when it loads one (`` !`…` ``, or a block opened with ```` ```! ````), or `hooks`, `mcpServers`, `permissionMode` or `allowed-tools` - in its front matter. A script beside a skill is taken: it runs only when the agent runs it, through the + in its front matter. Front matter is read as YAML (after a byte-order mark, flow style and merged keys + included); front matter that does not close, does not parse or is not a mapping is refused rather than guessed + at. A skill or command referring to a file with `@` that is not one of its own files is refused too: the agent + reads it into the prompt. A script beside a skill is taken: it runs only when the agent runs it, through the session's own rules. What was registered before stays: a tool server is still rendered, and may be unregistered; a hook or settings the diff --git a/modules/claude-code/cmd/claude-code/config.go b/modules/claude-code/cmd/claude-code/config.go index e638c949..fc8f5f05 100644 --- a/modules/claude-code/cmd/claude-code/config.go +++ b/modules/claude-code/cmd/claude-code/config.go @@ -41,6 +41,8 @@ import ( "strings" "sync" "time" + + "gopkg.in/yaml.v3" ) // Plugin is the plugin's name, and its marketplace's: what its items are called in a session, `nox-mesh:`. @@ -112,8 +114,14 @@ var runsFrontMatter = map[string]string{ // !`command` inline, or a fenced block opened with ```!. var shellLine = regexp.MustCompile("!`[^`\n]+`|(?m)^[ \t]*```!") +// fileRef is a file the agent may read into the prompt when it loads a skill or a command: `@` and a path, at the +// start of a line or after a space, a bracket or a quote. Any such word counts, a path without a dot or a slash +// (`@Makefile`) as much as one with: refused unless it is one of the item's own files, rather than guessed at. +var fileRef = regexp.MustCompile(`(?:^|[\s(\[{"'<])@([^\s` + "`" + `"')\]}>]+)`) + // runsProblem says why an item's text would run something in a session by itself, or "": the shell lines the -// agent runs when it loads a skill or a command, and the front matter keys that start, run or pre-approve. +// agent runs when it loads a skill or a command, the front matter keys that start, run or pre-approve, front +// matter the module cannot read for certain, and a file outside the item that the agent would read in. func runsProblem(it Item) string { if it.Kind != KindSkill && it.Kind != KindAgent && it.Kind != KindCommand { return "" @@ -127,13 +135,18 @@ func runsProblem(it Item) string { if !strings.HasSuffix(rel, ".md") { continue // a script beside a skill runs only when the agent runs it, through the session's own rules } - content := it.Files[rel] + content := strings.TrimPrefix(it.Files[rel], "\ufeff") if shellLine.MatchString(content) { return fmt.Sprintf("%s holds a shell line (!`…` or ```!), which the agent runs when it loads the %s, "+ "before any permission is asked; nothing registered through this module runs by itself in a "+ "session (novox/hq issue 340)", rel, it.Kind) } - for _, key := range frontMatterKeys(content) { + keys, problem := frontMatterKeys(content) + if problem != "" { + return fmt.Sprintf("%s: %s, so what it asks of a session cannot be judged; it is refused rather than "+ + "guessed at (novox/hq issue 340)", rel, problem) + } + for _, key := range keys { norm := strings.ToLower(strings.NewReplacer("-", "", "_", "").Replace(key)) if why, ok := runsFrontMatter[norm]; ok { return fmt.Sprintf("%s says %q in its front matter: %s. Nothing registered through this module "+ @@ -141,30 +154,64 @@ func runsProblem(it Item) string { "terminal (novox/hq issue 340)", rel, key, why) } } + if it.Kind == KindSkill || it.Kind == KindCommand { + for _, m := range fileRef.FindAllStringSubmatch(content, -1) { + ref := strings.TrimRight(m[1], ".,;:!?") + if ref == "" { + continue + } + if _, own := it.Files[ref]; !own || pathProblem(ref) != "" { + return fmt.Sprintf("%s refers to @%s, a file outside the %s that the agent reads into its "+ + "prompt when it loads it; a registered item refers only to its own files (novox/hq issue 340)", + rel, ref, it.Kind) + } + } + } } return "" } -// frontMatterKeys are the top-level keys of a markdown file's front matter: the lines between a first line -// `---` and the next, that start a key at the left margin. -func frontMatterKeys(content string) []string { - lines := strings.Split(strings.ReplaceAll(content, "\r\n", "\n"), "\n") - if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" { - return nil +// frontMatterKeys are the top-level keys of a markdown file's front matter, read as YAML: the block between a +// first line `---` (after a byte-order mark) and the next. A block that does not close, does not parse, or is not +// a mapping is a problem: the agent's own reader may take from it what this one cannot see. +func frontMatterKeys(content string) ([]string, string) { + content = strings.ReplaceAll(strings.TrimPrefix(content, "\ufeff"), "\r\n", "\n") + lines := strings.Split(content, "\n") + first := 0 + for first < len(lines) && strings.TrimSpace(lines[first]) == "" { + first++ } - var keys []string - for _, line := range lines[1:] { - if strings.TrimSpace(line) == "---" { + if first == len(lines) || strings.TrimSpace(lines[first]) != "---" { + return nil, "" + } + end := -1 + for i := first + 1; i < len(lines); i++ { + if t := strings.TrimSpace(lines[i]); t == "---" || t == "..." { + end = i break } - if line == "" || line[0] == ' ' || line[0] == '\t' || line[0] == '#' || line[0] == '-' { - continue - } - if i := strings.Index(line, ":"); i > 0 { - keys = append(keys, strings.Trim(strings.TrimSpace(line[:i]), `"'`)) - } } - return keys + if end < 0 { + return nil, "its front matter does not close" + } + block := strings.Join(lines[first+1:end], "\n") + if strings.TrimSpace(block) == "" { + return nil, "" + } + var node yaml.Node + if err := yaml.Unmarshal([]byte(block), &node); err != nil { + return nil, "its front matter is not YAML (" + err.Error() + ")" + } + var top map[string]any + if err := node.Decode(&top); err != nil { + return nil, "its front matter is not a mapping of keys (" + err.Error() + ")" + } + keys := make([]string, 0, len(top)) + for k := range top { + keys = append(keys, k) + } + sort.Strings(keys) + return keys, "" } // Item is one registration, as the state keeps it. diff --git a/modules/claude-code/cmd/claude-code/terminal_only_test.go b/modules/claude-code/cmd/claude-code/terminal_only_test.go index bf12401f..67828338 100644 --- a/modules/claude-code/cmd/claude-code/terminal_only_test.go +++ b/modules/claude-code/cmd/claude-code/terminal_only_test.go @@ -289,3 +289,53 @@ func TestThePluginsOnlyHookIsTheGuard(t *testing.T) { } } } + +// Front matter is read as the agent's own reader might read it (novox/hq issue 340): after a byte-order mark, in +// flow style, with quoted or merged keys. What does not parse, does not close, or is not a mapping is refused +// rather than guessed at. +func TestFrontMatterIsReadAsYAMLOrRefused(t *testing.T) { + skill := func(md string) Item { + return Item{Kind: KindSkill, Name: "x", Scope: ScopeMesh, Files: map[string]string{"SKILL.md": md}} + } + for label, md := range map[string]string{ + "a byte-order mark": "\ufeff---\nname: x\nallowed-tools: Bash\n---\nbody\n", + "a flow mapping": "---\n{name: x, allowed-tools: Bash}\n---\nbody\n", + "a quoted key": "---\nname: x\n\"hooks\": {}\n---\nbody\n", + "a merged key": "---\nbase: &b {hooks: {}}\n<<: *b\nname: x\n---\nbody\n", + "blank lines before it": "\n\n---\nname: x\nallowed_tools: Bash\n---\nbody\n", + "carriage returns": "---\r\nname: x\r\nallowed-tools: Bash\r\n---\r\nbody\r\n", + "no YAML": "---\nname: x\n bad: [\n---\nbody\n", + "not a mapping": "---\n- allowed-tools\n---\nbody\n", + "never closed": "---\nname: x\nallowed-tools: Bash\n", + "a duplicate key": "---\nname: x\nname: y\n---\nbody\n", + } { + if problem := skill(md).Problem(); !strings.Contains(problem, "issue 340") { + t.Errorf("%s was taken: %q", label, problem) + } + } + if problem := skill("---\nname: x\ndescription: \"a: b\"\nwhen_to_use: [a, b]\n---\nbody\n").Problem(); problem != "" { + t.Errorf("plain front matter was refused: %s", problem) + } +} + +// A skill or a command refers only to its own files (novox/hq issue 340): `@path` reads a file into the prompt when +// the item loads, so one outside it would put another file's words, or a secret, in every session it reaches. +func TestAFileReferenceOutsideTheItemIsRefused(t *testing.T) { + for _, ref := range []string{"@~/.ssh/id_ed25519", "@/etc/shadow", "@../other/SKILL.md", "@README.md", "(@./notes.md)", + "@scripts/../../x.sh", "@Makefile", "@$HOME/.netrc"} { + it := Item{Kind: KindSkill, Name: "x", Scope: ScopeMesh, Files: map[string]string{ + "SKILL.md": "---\nname: x\n---\nRead " + ref + " first.\n", "scripts/run.sh": "#!/bin/sh\n"}} + if problem := it.Problem(); !strings.Contains(problem, "outside") { + t.Errorf("%s was taken: %q", ref, problem) + } + cmd := Item{Kind: KindCommand, Name: "x", Scope: ScopeHome, Files: one("x", "Look at "+ref)} + if problem := cmd.Problem(); !strings.Contains(problem, "outside") { + t.Errorf("%s in a command was taken: %q", ref, problem) + } + } + own := Item{Kind: KindSkill, Name: "x", Scope: ScopeMesh, Files: map[string]string{ + "SKILL.md": "---\nname: x\n---\nRun @scripts/run.sh. Mail jochen@example.org.\n", "scripts/run.sh": "#!/bin/sh\n"}} + if problem := own.Problem(); problem != "" { + t.Errorf("a reference to its own file or a mail address was refused: %s", problem) + } +} diff --git a/modules/claude-code/go.mod b/modules/claude-code/go.mod index 16f3f8a6..9758de50 100644 --- a/modules/claude-code/go.mod +++ b/modules/claude-code/go.mod @@ -3,3 +3,5 @@ module claude-code go 1.25.0 require git.novox.be/novox/mesh-sdk/go v0.1.7 + +require gopkg.in/yaml.v3 v3.0.1 diff --git a/modules/claude-code/go.sum b/modules/claude-code/go.sum index b474419e..3e4a7c35 100644 --- a/modules/claude-code/go.sum +++ b/modules/claude-code/go.sum @@ -1,2 +1,6 @@ git.novox.be/novox/mesh-sdk/go v0.1.7 h1:C0sTQmtTiyYH7bnqZb7PusXnqA37gKuT7Nqjn9gG47w= git.novox.be/novox/mesh-sdk/go v0.1.7/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=