From 4eb4bcfc507e498952352a000942eb8bbe6a6605 Mon Sep 17 00:00:00 2001
From: Alex Dunmow <alex@dx314.com>
Date: Thu, 9 Jul 2026 09:48:54 +0800
Subject: [PATCH] fix(plugin): writeMod drops
 allowed_hosts/max_response_mb/required_icon_packs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

writeMod hand-reconstructs plugin.mod field by field but never emitted three
first-class core.ModFile fields, so ParseModFull → mutate → writeMod (every
bump/init/tags edit) silently stripped them. For allowed_hosts this is
security-relevant: a `ninja plugin bump` erased a plugin's ADR-0023 egress
declaration, the install/update consent gate saw nothing to grant, and the
plugin's outbound calls were denied at runtime with no prompt ever shown.

Emit all three; add a writeMod ⇄ ParseModFull round-trip regression test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 cmd/ninja/cmd/plugin.go      | 17 ++++++++++++++++
 cmd/ninja/cmd/plugin_test.go | 39 ++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/cmd/ninja/cmd/plugin.go b/cmd/ninja/cmd/plugin.go
index 2b670d0..c39825f 100644
--- a/cmd/ninja/cmd/plugin.go
+++ b/cmd/ninja/cmd/plugin.go
@@ -1107,12 +1107,29 @@ func writeMod(path string, m *core.ModFile) error {
 		}
 		fmt.Fprintf(&b, "tags = [%s]\n", strings.Join(quoted, ", "))
 	}
+	if len(m.Plugin.RequiredIconPacks) > 0 {
+		quoted := make([]string, len(m.Plugin.RequiredIconPacks))
+		for i, p := range m.Plugin.RequiredIconPacks {
+			quoted[i] = fmt.Sprintf("%q", p)
+		}
+		fmt.Fprintf(&b, "required_icon_packs = [%s]\n", strings.Join(quoted, ", "))
+	}
 	if m.Plugin.Private {
 		b.WriteString("private = true\n")
 	}
 	if m.Plugin.DataDir {
 		b.WriteString("data_dir = true\n")
 	}
+	if len(m.Plugin.AllowedHosts) > 0 {
+		quoted := make([]string, len(m.Plugin.AllowedHosts))
+		for i, h := range m.Plugin.AllowedHosts {
+			quoted[i] = fmt.Sprintf("%q", h)
+		}
+		fmt.Fprintf(&b, "allowed_hosts = [%s]\n", strings.Join(quoted, ", "))
+	}
+	if m.Plugin.MaxResponseMB > 0 {
+		fmt.Fprintf(&b, "max_response_mb = %d\n", m.Plugin.MaxResponseMB)
+	}
 	if m.Compatibility != nil {
 		b.WriteString("\n[compatibility]\n")
 		fmt.Fprintf(&b, "block_core = %q\n", m.Compatibility.BlockCore)
diff --git a/cmd/ninja/cmd/plugin_test.go b/cmd/ninja/cmd/plugin_test.go
index 8bad976..8020c49 100644
--- a/cmd/ninja/cmd/plugin_test.go
+++ b/cmd/ninja/cmd/plugin_test.go
@@ -364,6 +364,45 @@ func TestWriteMod_PrivateFalseOmitted(t *testing.T) {
 	}
 }
 
+// TestWriteMod_RoundTripsFirstClassFields guards the writeMod ⇄ ParseModFull
+// round-trip for the first-class fields the hand-rolled writer must emit: a
+// bump/init/tags edit re-serializes plugin.mod, and any field the writer forgets
+// is silently dropped. allowed_hosts (ADR 0023) dropping is a security-relevant
+// regression — a bump strips the egress declaration and the consent gate goes
+// dark — so this test exists to keep the writer in sync with core.ModFile.
+func TestWriteMod_RoundTripsFirstClassFields(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "plugin.mod")
+	m := &core.ModFile{Plugin: core.ModPlugin{
+		Name:              "myplugin",
+		Scope:             "ninja",
+		Version:           "1.2.3",
+		AllowedHosts:      []string{"api.cal.com", "*.example.com"},
+		MaxResponseMB:     25,
+		RequiredIconPacks: []string{"tabler"},
+	}}
+	if err := writeMod(path, m); err != nil {
+		t.Fatalf("writeMod: %v", err)
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	back, err := core.ParseModFull(data)
+	if err != nil {
+		t.Fatalf("ParseModFull round-trip: %v\n%s", err, data)
+	}
+	if got := back.Plugin.AllowedHosts; len(got) != 2 || got[0] != "api.cal.com" || got[1] != "*.example.com" {
+		t.Errorf("allowed_hosts not preserved: %v\n%s", got, data)
+	}
+	if back.Plugin.MaxResponseMB != 25 {
+		t.Errorf("max_response_mb not preserved: %d\n%s", back.Plugin.MaxResponseMB, data)
+	}
+	if got := back.Plugin.RequiredIconPacks; len(got) != 1 || got[0] != "tabler" {
+		t.Errorf("required_icon_packs not preserved: %v\n%s", got, data)
+	}
+}
+
 func TestParsePrivateCoord(t *testing.T) {
 	cases := []struct {
 		in      string