fix: emit safe skill frontmatter YAML and verify it round-trips

Descriptions written through the API were interpolated raw into the SKILL.md
frontmatter, so a value containing a colon+space made the whole frontmatter
unparseable: every field silently stayed empty, the skill loaded under its
directory name, and a skill-gated MCP server was silently skipped. The same
applied to updates through the meta rewrite, and the shipped scopesentry skill
had a malformed name line all along.

- yamlScalar: plain-safe tokens stay bare, everything else single-quoted
- fsCreateSkill/fsUpdateSkillMeta emit safe values and re-verify the written
  file with the real skill parser (name + mcps must survive), rejecting the
  write otherwise
- warnBrokenSkills at startup logs skills whose name fell back to the
  directory (the signature of broken frontmatter)
- fix skills/scopesentry/SKILL.md name line

Closes #39

.gitignore

@@ -57,4 +57,4 @@ Thumbs.db
 # bench benchmark related (local artifacts, not committed)
 /bench/
 /Dockerfile.bench
-/Dockerfile.bench.v2backups/
+/Dockerfile.bench.v2

README.md

@@ -148,29 +148,6 @@ ARTEX_TARGETS=linux/amd64,windows/amd64 ./build.sh --release
 
 ---
 
-## Backups
-
-`scripts/backup.sh` dumps the Postgres database (and optionally the data
-directory) with timestamped output and rotation:
-
-```bash
-scripts/backup.sh --with-data          # db dump + data/ archive, keep 14
-scripts/backup.sh --keep 7             # db only, keep 7
-```
-
-Restore a dump into a running stack:
-
-```bash
-gunzip -c backups/artex-db-<timestamp>.sql.gz | docker compose exec -T postgres \
-  psql -U "$POSTGRES_USER" -d "$POSTGRES_DB"
-```
-
-Daily cron example (03:20, operator crontab):
-
-```
-20 3 * * * cd /path/to/ARTEX && scripts/backup.sh --with-data >> backups/backup.log 2>&1
-```
-
 ## Updating and upgrading
 
 > Upgrading only swaps the program and never touches data: the Postgres volume `pgdata`, `./data` (jwt.key / SQLite, etc.), and `./skills` are all preserved. **No manual database migration is needed**: `artex` idempotently re-runs `schema.sql` on every start (with `ADD COLUMN` / `CREATE INDEX IF NOT EXISTS`), i.e. "restart equals migrate". It is still recommended to back up `./data` and the database before upgrading.

scripts/backup.sh

@@ -1,85 +0,0 @@
-#!/usr/bin/env bash
-# Backup the ARTEX database (and optionally the data directory) with rotation.
-#
-# Usage:
-#   scripts/backup.sh [--with-data] [--keep N] [--output-dir DIR]
-#
-#   --with-data    also archive the bind-mounted data/ directory (worker
-#                  artifacts, evidence, skills state) as a tar.gz
-#   --keep N       number of db dumps to keep (default 14; data archives rotate
-#                  to the same count)
-#   --output-dir   where to write (default backups/, relative to the repo root)
-#
-# The database dump runs inside the postgres compose service, so no psql client
-# is needed on the host. Restore:
-#   gunzip -c backups/artex-db-<ts>.sql.gz | docker compose exec -T postgres \
-#     psql -U "$POSTGRES_USER" -d "$POSTGRES_DB"
-set -euo pipefail
-cd "$(dirname "$0")/.."
-
-usage() { sed -n '2,15p' "$0" | sed 's/^# \?//'; }
-
-KEEP=14
-OUTPUT_DIR="backups"
-WITH_DATA=0
-while [ $# -gt 0 ]; do
-  case "$1" in
-    --keep) KEEP="${2:?}"; shift 2 ;;
-    --output-dir) OUTPUT_DIR="${2:?}"; shift 2 ;;
-    --with-data) WITH_DATA=1; shift ;;
-    -h|--help) usage; exit 0 ;;
-    *) echo "unknown option: $1" >&2; usage >&2; exit 2 ;;
-  esac
-done
-case "$KEEP" in (*[!0-9]*|'') echo "--keep must be a positive integer" >&2; exit 2 ;; esac
-
-# Compose reads .env for credentials; source it for the dump command too.
-if [ -f .env ]; then
-  set -a
-  # shellcheck disable=SC1091
-  . ./.env
-  set +a
-fi
-PGUSER_NAME="${POSTGRES_USER:-artex}"
-PGDATABASE_NAME="${POSTGRES_DB:-artex}"
-
-mkdir -p "$OUTPUT_DIR"
-TS=$(date -u +%Y%m%dT%H%M%SZ)
-DB_OUT="$OUTPUT_DIR/artex-db-$TS.sql.gz"
-if [ -e "$DB_OUT" ]; then
-  echo "refusing to overwrite $DB_OUT" >&2
-  exit 1
-fi
-
-if ! docker compose exec -T postgres pg_dump -U "$PGUSER_NAME" "$PGDATABASE_NAME" | gzip > "$DB_OUT"; then
-  rm -f "$DB_OUT"
-  echo "pg_dump failed (is the stack running?)" >&2
-  exit 1
-fi
-gzip -t "$DB_OUT"
-echo "wrote $DB_OUT ($(du -h "$DB_OUT" | cut -f1))"
-
-if [ "$WITH_DATA" -eq 1 ]; then
-  DATA_OUT="$OUTPUT_DIR/artex-data-$TS.tar.gz"
-  # data/ contains files written by the container (root); unreadable ones make
-  # tar exit 2. Fail loudly rather than ship a partial archive.
-  if ! tar -czf "$DATA_OUT" data 2>"$OUTPUT_DIR/.tar.err"; then
-    rm -f "$DATA_OUT"
-    echo "data archive failed (run as root so container-written files are readable):" >&2
-    head -5 "$OUTPUT_DIR/.tar.err" >&2
-    rm -f "$OUTPUT_DIR/.tar.err"
-    exit 1
-  fi
-  rm -f "$OUTPUT_DIR/.tar.err"
-  echo "wrote $DATA_OUT ($(du -h "$DATA_OUT" | cut -f1))"
-fi
-
-prune() {
-  ls -1t "$OUTPUT_DIR"/$1 2>/dev/null | tail -n +$((KEEP + 1)) | while IFS= read -r old; do
-    echo "pruning $old"
-    rm -f "$old"
-  done
-}
-prune 'artex-db-*.sql.gz'
-[ "$WITH_DATA" -eq 1 ] && prune 'artex-data-*.tar.gz'
-echo "backup complete ($KEEP newest kept)"

server/server.go

@@ -219,9 +219,10 @@ func New(ctx context.Context, m *Manager, skillDir string, dataDir string, keyDi
 		}
 		wireAgentAugment(m.pg, s.skillDir, s.hostTools) // wire visible skills/MCP + traffic/orchestration host tools into the agent toolset
 		domainReg := buildDomainReg(m.Assets())
-		wireTools(m.pg, domainReg) // built-in tools table: filter per agent + override description/schema + inject defaults
-		seedPrompts(m.pg)          // seed built-in agent default prompt bodies into agent_prompts (only when empty)
-		s.seedOrchestrationTools() // seed P2 cross-task orchestration tools into the tools table (bindable per agent)
+		wireTools(m.pg, domainReg)   // built-in tools table: filter per agent + override description/schema + inject defaults
+		seedPrompts(m.pg)            // seed built-in agent default prompt bodies into agent_prompts (only when empty)
+		warnBrokenSkills(s.skillDir) // surface skills whose frontmatter silently failed to parse
+		s.seedOrchestrationTools()   // seed P2 cross-task orchestration tools into the tools table (bindable per agent)
 		if err := s.seedFindingRetester(); err != nil {
 			log.Printf("[retester] seed: %v", err)
 		}

server/server_mgmt.go

@@ -1108,17 +1108,21 @@ func (s *Server) fsCreateSkill(w http.ResponseWriter, r *http.Request) {
 	//   Markdown body:    step-by-step instructions
 	var sb strings.Builder
 	sb.WriteString("---\n")
-	fmt.Fprintf(&sb, "name: %s\n", body.Name)
-	fmt.Fprintf(&sb, "description: %s\n", body.Description)
+	fmt.Fprintf(&sb, "name: %s\n", yamlScalar(body.Name))
+	fmt.Fprintf(&sb, "description: %s\n", yamlScalar(body.Description))
 	if body.License != "" {
-		fmt.Fprintf(&sb, "license: %s\n", body.License)
+		fmt.Fprintf(&sb, "license: %s\n", yamlScalar(body.License))
 	}
 	if body.Compatibility != "" {
-		fmt.Fprintf(&sb, "compatibility: %s\n", body.Compatibility)
+		fmt.Fprintf(&sb, "compatibility: %s\n", yamlScalar(body.Compatibility))
 	}
 	// mcps: MCP servers this skill unlocks on load (skill-gated deferred tools).
 	if mcps := cleanStrs(body.MCPs); len(mcps) > 0 {
-		fmt.Fprintf(&sb, "mcps: %s\n", strings.Join(mcps, ", "))
+		quoted := make([]string, len(mcps))
+		for i, m := range mcps {
+			quoted[i] = yamlScalar(m)
+		}
+		fmt.Fprintf(&sb, "mcps: %s\n", strings.Join(quoted, ", "))
 	}
 	sb.WriteString("---\n")
 	if strings.TrimSpace(body.Instructions) != "" {
@@ -1134,6 +1138,14 @@ func (s *Server) fsCreateSkill(w http.ResponseWriter, r *http.Request) {
 		writeErr(w, 500, err.Error())
 		return
 	}
+	// Verify with the real parser: a description containing ": " (or any value that
+	// is not plain-safe YAML) used to break the whole frontmatter silently, leaving
+	// a skill whose fields were all empty and whose name came from the directory.
+	if err := verifySkillFile(skillPath, body.Name, cleanStrs(body.MCPs)); err != nil {
+		_ = os.RemoveAll(skillPath)
+		writeErr(w, 400, "generated SKILL.md does not round-trip through the skill parser: "+err.Error())
+		return
+	}
 	writeJSON(w, 201, map[string]any{"name": body.Name})
 }
 
@@ -1176,6 +1188,54 @@ func (s *Server) fsUpdateSkillMeta(w http.ResponseWriter, r *http.Request) {
 	writeJSON(w, 200, map[string]any{"ok": true})
 }
 
+// yamlScalar renders one frontmatter scalar so the emitted YAML is valid by
+// construction: plain-safe tokens stay bare, everything else is single-quoted
+// with internal quotes doubled (YAML single-quote escaping).
+func yamlScalar(s string) string {
+	if s != "" && !strings.ContainsAny(s, ":#{}[]&*!|>'\"%@`,\n") && !strings.HasPrefix(s, " ") && !strings.HasSuffix(s, " ") {
+		return s
+	}
+	return "'" + strings.ReplaceAll(s, "'", "''") + "'"
+}
+
+// verifySkillFile re-loads a written SKILL.md with the real skill parser and
+// asserts the name and the mcps list survived the round trip.
+func verifySkillFile(skillPath, wantName string, wantMCPs []string) error {
+	reg, err := skill.LoadDir(skillPath)
+	if err != nil {
+		return fmt.Errorf("load: %w", err)
+	}
+	for _, s := range reg.List() {
+		if s.Dir == "" {
+			continue
+		}
+		if s.Name != wantName {
+			return fmt.Errorf("parsed name %q != written %q (frontmatter did not parse; the name fell back to the directory)", s.Name, wantName)
+		}
+		if len(wantMCPs) > 0 && len(s.MCPs) != len(wantMCPs) {
+			return fmt.Errorf("parsed mcps %v != written %v", s.MCPs, wantMCPs)
+		}
+		return nil
+	}
+	return fmt.Errorf("no skill loaded from %s", skillPath)
+}
+
+// warnBrokenSkills logs skills whose parsed name does not match their directory
+// name: the signature of frontmatter that failed to parse and silently fell back
+// (fields empty, mcps gate lost). Best-effort, startup only.
+func warnBrokenSkills(dir string) {
+	reg, err := skill.LoadDir(dir)
+	if err != nil {
+		return
+	}
+	for _, s := range reg.List() {
+		if s.Dir == "" || s.Name == filepath.Base(s.Dir) {
+			continue
+		}
+		log.Printf("[skill] %s: frontmatter did not parse (loaded under the directory name; description and mcps are empty) - fix the YAML in %s", s.Name, filepath.Join(s.Dir, "SKILL.md"))
+	}
+}
+
 // rewriteSkillFrontmatter parses the SKILL.md YAML frontmatter and replaces the
 // fields given as non-nil pointers. The instruction body (after the closing ---) is
 // preserved verbatim. Returns an error if the file has no recognisable frontmatter.
@@ -1211,11 +1271,11 @@ func rewriteSkillFrontmatter(content []byte, mcps *[]string, description, licens
 		}
 		for i, p := range pairs {
 			if p.k == key {
-				pairs[i].v = strings.TrimSpace(*val)
+				pairs[i].v = yamlScalar(strings.TrimSpace(*val))
 				return
 			}
 		}
-		pairs = append(pairs, kv{key, strings.TrimSpace(*val)})
+		pairs = append(pairs, kv{key, yamlScalar(strings.TrimSpace(*val))})
 	}
 	applyStr("description", description)
 	applyStr("license", license)
@@ -1231,7 +1291,11 @@ func rewriteSkillFrontmatter(content []byte, mcps *[]string, description, licens
 		}
 		pairs = filtered
 		if len(cleaned) > 0 {
-			pairs = append(pairs, kv{"mcps", strings.Join(cleaned, ", ")})
+			quoted := make([]string, len(cleaned))
+			for i, m := range cleaned {
+				quoted[i] = yamlScalar(m)
+			}
+			pairs = append(pairs, kv{"mcps", strings.Join(quoted, ", ")})
 		}
 	}
 	// Reconstruct

server/skill_frontmatter_test.go

@@ -0,0 +1,72 @@
+package server
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/Autumn-27/norma/skill"
+)
+
+func TestYamlScalar(t *testing.T) {
+	cases := []struct{ in, want string }{
+		{"rea", "rea"},
+		{"api-recon", "api-recon"},
+		{"plain value with spaces", "plain value with spaces"},
+		{"target: embedded", "'target: embedded'"},
+		{"it's quoted", "'it''s quoted'"},
+		{"ends #now", "'ends #now'"},
+		{"comma, separated", "'comma, separated'"},
+		{" leading", "' leading'"},
+		{"", "''"},
+	}
+	for _, c := range cases {
+		if got := yamlScalar(c.in); got != c.want {
+			t.Errorf("yamlScalar(%q) = %q, want %q", c.in, got, c.want)
+		}
+	}
+}
+
+// A SKILL.md written the way fsCreateSkill now writes it (yamlScalar values)
+// must round-trip through the real parser with name and mcps intact, even with
+// a colon+space and quotes in the description. The pre-fix format (raw
+// interpolation) fails exactly there and falls back to the directory name.
+func TestSkillFrontmatterRoundTrip(t *testing.T) {
+	dir := t.TempDir()
+	skillDir := filepath.Join(dir, "colon-skill")
+	if err := os.MkdirAll(skillDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	desc := "Analyze target: embedded secrets, it's binary"
+	content := fmt.Sprintf("---\nname: %s\ndescription: %s\nmcps: %s\n---\nbody instructions\n",
+		yamlScalar("colon-skill"), yamlScalar(desc), yamlScalar("rea"))
+
+	skillMD := filepath.Join(skillDir, "SKILL.md")
+	if err := os.WriteFile(skillMD, []byte(content), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := verifySkillFile(skillDir, "colon-skill", []string{"rea"}); err != nil {
+		t.Fatalf("safe format failed verification: %v", err)
+	}
+	reg, err := skill.LoadDir(skillDir)
+	if err != nil || len(reg.List()) != 1 {
+		t.Fatalf("LoadDir: %v (%d skills)", err, len(reg.List()))
+	}
+	s := reg.List()[0]
+	if s.Name != "colon-skill" || s.Description != desc || len(s.MCPs) != 1 || s.MCPs[0] != "rea" {
+		t.Fatalf("round-trip mismatch: name=%q desc=%q mcps=%v", s.Name, s.Description, s.MCPs)
+	}
+
+	// the old, unsafe emission: unquoted colon+space breaks the whole frontmatter
+	unsafe := fmt.Sprintf("---\nname: %s\ndescription: %s\nmcps: rea\n---\nbody\n", "colon-skill", desc)
+	if err := os.WriteFile(skillMD, []byte(unsafe), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := verifySkillFile(skillDir, "colon-skill", []string{"rea"}); err == nil {
+		t.Fatal("unsafe format unexpectedly verified")
+	} else if !strings.Contains(err.Error(), "fell back") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}

skills/scopesentry/SKILL.md

@@ -1,6 +1,6 @@
 ---
 
-## name: scopesentry-mcp
+name: scopesentry-mcp
 description: Manage the security scanning platform (projects, tasks, templates, assets, nodes) through the ScopeSentry MCP. Use when the user mentions ScopeSentry, MCP, API Key, scan tasks, or asset queries.
 
 # ScopeSentry MCP usage guide