Skip to content

Commit e961d2f

Browse files
committed
skillinject: per-tool webhookRoutes block + YAML merge
New ManifestWebhookRoute schema lets a tool entry declare any number of routes the daemon merges into a target YAML config. Today this is hermes-only — hermes-agent exposes a first-class webhook receiver at platforms.webhook.extra.routes; we drop a "pilot-events" entry there so pilot-daemon can POST inbox/file/trust events directly. The YAML merge follows the same 6-step safety contract as the JSON plugin-allowlist merge: in-memory snapshot, refuse-on-parse-failure, pre-write verification, .pilot-bak sidecar, atomic .tmp rename, post-swap verification with rollback. yamlEqual normalizes both sides through a marshal round-trip so int/float drift across user-edited vs manifest-provided values doesn't trigger spurious rewrites. Comment-preservation caveat: the current implementation parses to a generic map and re-marshals, so inline comments on touched nodes are lost. Upgrading to yaml.Node-mode editing is a future cleanup — documented inline. Tests cover create-when-missing, preserve-other-keys, idempotent reconcile (no rewrite when state matches), refuse-on-malformed, and manifest-schema round-trip.
1 parent 4e2513f commit e961d2f

5 files changed

Lines changed: 548 additions & 0 deletions

File tree

internal/skillinject/manifest.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,45 @@ type ManifestTool struct {
7171
// to a single Plugin block. If both Plugin and Plugins are set, the
7272
// daemon reconciles Plugin first then iterates Plugins.
7373
Plugins []ManifestPlugin `json:"plugins,omitempty"`
74+
// WebhookRoutes is the per-tool list of routes the daemon merges
75+
// into a YAML config (today hermes — see ManifestWebhookRoute). One
76+
// tool can declare any number; each reconciles independently.
77+
WebhookRoutes []ManifestWebhookRoute `json:"webhookRoutes,omitempty"`
78+
}
79+
80+
// ManifestWebhookRoute describes a route the daemon adds to a YAML
81+
// config file so the tool's built-in webhook receiver accepts pilot
82+
// events at a known path. Modeled on hermes-agent's
83+
// platforms.webhook.extra.routes schema (one named route entry per
84+
// agent integration, HMAC-signed, optional event allow-list,
85+
// optional prompt template).
86+
//
87+
// The daemon owns the named route — operators should not hand-edit
88+
// the same RouteName under RoutesYamlPath; the reconcile loop will
89+
// overwrite it. Other keys in the file (including other routes) are
90+
// preserved.
91+
//
92+
// Comment-preservation caveat: the current implementation parses YAML
93+
// to a generic map and re-marshals, which loses inline comments on
94+
// any node it touches. Comments on keys outside the modified subtree
95+
// survive only if the YAML library happens to preserve them across
96+
// the round-trip — yaml.v3's default does not. A future upgrade to
97+
// yaml.Node-mode editing would close this gap.
98+
type ManifestWebhookRoute struct {
99+
// ConfigPath is the YAML file the daemon merges into
100+
// (e.g. "~/.hermes/config.yaml").
101+
ConfigPath string `json:"configPath"`
102+
// RoutesYamlPath is the dotted path to the routes map. Created if
103+
// absent; intermediate maps are materialized.
104+
// e.g. "platforms.webhook.extra.routes".
105+
RoutesYamlPath string `json:"routesYamlPath"`
106+
// RouteName is the key under RoutesYamlPath where our entry goes
107+
// (e.g. "pilot-events"). The daemon owns this key.
108+
RouteName string `json:"routeName"`
109+
// Route is the YAML body to assign at RoutesYamlPath.RouteName.
110+
// Free-form so future schema additions don't require a Go change —
111+
// the daemon passes it through verbatim.
112+
Route map[string]interface{} `json:"route"`
74113
}
75114

76115
// ManifestPlugin describes a per-tool plugin that the daemon writes

internal/skillinject/skillinject.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,14 @@ func Tick(ctx context.Context, cfg Config) (*Report, error) {
237237
reconcilePluginAllowList(p, home))
238238
}
239239
}
240+
241+
// Webhook-route slot — one entry per route the daemon merges
242+
// into the tool's YAML config. Today this is hermes-only
243+
// (config.yaml under platforms.webhook.extra.routes).
244+
for i := range mt.WebhookRoutes {
245+
report.Outcomes = append(report.Outcomes,
246+
reconcileWebhookRoute(&mt.WebhookRoutes[i], home))
247+
}
240248
}
241249

242250
return report, nil
@@ -278,6 +286,27 @@ func reconcilePluginFiles(f *fetcher, ctx context.Context, p *ManifestPlugin, ho
278286
return out
279287
}
280288

289+
// reconcileWebhookRoute merges a named route into a YAML config so the
290+
// tool's webhook receiver accepts pilot events. Single Outcome per
291+
// route; the path field points at the YAML file mutated. Same
292+
// classify→action→merge shape as plugin reconciliation; the YAML merge
293+
// itself follows the 6-step safety contract in mergeWebhookRoute.
294+
func reconcileWebhookRoute(r *ManifestWebhookRoute, home string) Outcome {
295+
cfgPath := expandHome(r.ConfigPath, home)
296+
o := Outcome{Tool: r.RouteName, Kind: KindWebhookRoute, Path: cfgPath}
297+
state := classifyWebhookRoute(cfgPath, r.RoutesYamlPath, r.RouteName, r.Route)
298+
o.State = state
299+
o.Action = actionFor(state)
300+
if o.Action == ActionNoop {
301+
return o
302+
}
303+
if err := mergeWebhookRoute(cfgPath, r.RoutesYamlPath, r.RouteName, r.Route); err != nil {
304+
o.Action = ActionError
305+
o.Err = err.Error()
306+
}
307+
return o
308+
}
309+
281310
// reconcilePluginAllowList does a JSON-merge into the tool's plugin
282311
// config so the plugin id appears in the trust array AND its entries
283312
// row has enabled=true. Single Outcome; the path field points at the

internal/skillinject/state.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const (
3939
KindHelper FileKind = "helper"
4040
KindPluginFile FileKind = "plugin_file"
4141
KindPluginAllowList FileKind = "plugin_allowlist"
42+
KindWebhookRoute FileKind = "webhook_route"
4243
)
4344

4445
func actionFor(s State) Action {
Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+
package skillinject
4+
5+
import (
6+
"bytes"
7+
"fmt"
8+
"os"
9+
"path/filepath"
10+
"reflect"
11+
"strings"
12+
13+
"gopkg.in/yaml.v3"
14+
)
15+
16+
// classifyWebhookRoute inspects the target YAML config and reports
17+
// whether the named route at the dotted path already matches the
18+
// desired Route body.
19+
//
20+
// - StateIdentical: route exists at the path and DeepEqual to the
21+
// manifest's body. No write needed.
22+
// - StateDrifted: route exists but differs, OR the file exists but
23+
// the path / entry is absent. Daemon merges + rewrites.
24+
// - StateDrifted (with create-on-write semantics): file missing.
25+
// Same response — the merge function creates it.
26+
//
27+
// Same idiom as classifyPluginAllowList: read-only inspection here,
28+
// the mutation lives in mergeWebhookRoute.
29+
func classifyWebhookRoute(configPath, routesYamlPath, routeName string, want map[string]interface{}) State {
30+
raw, err := os.ReadFile(configPath)
31+
if err != nil {
32+
// File absent or unreadable → drifted (merge will handle both
33+
// cases). Caller already gated on dirExists for the tool root.
34+
return StateDrifted
35+
}
36+
var obj map[string]any
37+
if err := yaml.Unmarshal(raw, &obj); err != nil {
38+
// Unparseable config — never overwrite a malformed user file.
39+
// Return Drifted so the caller surfaces the error in the next
40+
// tick's outcome (the merge step will refuse with a clear msg).
41+
return StateDrifted
42+
}
43+
if obj == nil {
44+
return StateDrifted
45+
}
46+
got := lookupYamlPath(obj, routesYamlPath, routeName)
47+
if got == nil {
48+
return StateDrifted
49+
}
50+
// Normalize both sides through yaml round-trip to avoid spurious
51+
// drift from e.g. int vs uint type variance between user-edited
52+
// and manifest-provided YAML.
53+
if !yamlEqual(got, want) {
54+
return StateDrifted
55+
}
56+
return StateIdentical
57+
}
58+
59+
// mergeWebhookRoute writes the named route into the tool's YAML
60+
// config, preserving every other top-level key. Follows the same
61+
// 6-step safety contract as mergePluginAllowList — see that comment
62+
// for the full rationale.
63+
func mergeWebhookRoute(configPath, routesYamlPath, routeName string, want map[string]interface{}) error {
64+
// (1) Snapshot the original bytes in memory.
65+
var (
66+
originalBytes []byte
67+
originalExisted bool
68+
obj map[string]any
69+
)
70+
raw, err := os.ReadFile(configPath)
71+
switch {
72+
case err != nil && os.IsNotExist(err):
73+
obj = map[string]any{}
74+
case err != nil:
75+
return fmt.Errorf("read webhook-route config: %w", err)
76+
default:
77+
originalBytes = append([]byte(nil), raw...)
78+
originalExisted = true
79+
// (2) Refuse to operate on a malformed config.
80+
if uerr := yaml.Unmarshal(raw, &obj); uerr != nil {
81+
return fmt.Errorf("parse webhook-route config (refusing to overwrite a malformed user config): %w", uerr)
82+
}
83+
if obj == nil {
84+
obj = map[string]any{}
85+
}
86+
}
87+
88+
originalTopKeys := make(map[string]struct{}, len(obj))
89+
for k := range obj {
90+
originalTopKeys[k] = struct{}{}
91+
}
92+
93+
if err := ensureWebhookRouteEntry(obj, routesYamlPath, routeName, want); err != nil {
94+
return err
95+
}
96+
97+
next, err := marshalYAMLStable(obj)
98+
if err != nil {
99+
return fmt.Errorf("marshal merged webhook-route config: %w", err)
100+
}
101+
102+
// (3) Pre-write verification: round-trip the bytes we're about to
103+
// commit and confirm (a) they parse, (b) every original top-level
104+
// key is still present, (c) our route is present and equal to the
105+
// manifest's want.
106+
if err := verifyWebhookRouteRoundTrip(next, originalTopKeys, routesYamlPath, routeName, want); err != nil {
107+
return fmt.Errorf("pre-write verification failed (refusing to write): %w", err)
108+
}
109+
110+
if err := os.MkdirAll(filepath.Dir(configPath), 0o755); err != nil {
111+
return fmt.Errorf("ensure parent dir for webhook-route config: %w", err)
112+
}
113+
114+
// (4) Sidecar backup of the original bytes if they differ. Re-uses
115+
// the BackupSuffix constant + writeFileAtomic helper from
116+
// plugin_allowlist.go so all skillinject-managed configs use the
117+
// same .pilot-bak rotation pattern.
118+
if originalExisted && !bytes.Equal(originalBytes, next) {
119+
bakPath := configPath + BackupSuffix
120+
if err := writeFileAtomic(bakPath, originalBytes, 0o644); err != nil {
121+
return fmt.Errorf("write pre-merge backup: %w", err)
122+
}
123+
}
124+
125+
// (5) Atomic swap.
126+
if err := writeFileAtomic(configPath, next, 0o644); err != nil {
127+
return fmt.Errorf("write webhook-route config: %w", err)
128+
}
129+
130+
// (6) Post-swap verification with rollback on failure.
131+
if err := verifyWebhookRouteOnDisk(configPath, originalTopKeys, routesYamlPath, routeName, want); err != nil {
132+
if originalExisted {
133+
if rbErr := writeFileAtomic(configPath, originalBytes, 0o644); rbErr != nil {
134+
return fmt.Errorf("post-write verification failed (%v); ROLLBACK ALSO FAILED (%v); manual restore: cp %s%s %s",
135+
err, rbErr, configPath, BackupSuffix, configPath)
136+
}
137+
return fmt.Errorf("post-write verification failed; rolled back from in-memory snapshot: %w", err)
138+
}
139+
return fmt.Errorf("post-write verification failed (no rollback — no original existed): %w", err)
140+
}
141+
return nil
142+
}
143+
144+
// marshalYAMLStable marshals via yaml.v3 with 2-space indent (yaml's
145+
// default). yaml.v3 doesn't guarantee map-key ordering across runs,
146+
// but for our use case the daemon's reconcile loop is idempotent —
147+
// subsequent ticks compare deep-equal and skip if want == got, so a
148+
// shuffled order between identical writes is benign.
149+
func marshalYAMLStable(obj map[string]any) ([]byte, error) {
150+
var buf bytes.Buffer
151+
enc := yaml.NewEncoder(&buf)
152+
enc.SetIndent(2)
153+
if err := enc.Encode(obj); err != nil {
154+
return nil, err
155+
}
156+
if err := enc.Close(); err != nil {
157+
return nil, err
158+
}
159+
return buf.Bytes(), nil
160+
}
161+
162+
func verifyWebhookRouteRoundTrip(b []byte, originalTopKeys map[string]struct{}, routesYamlPath, routeName string, want map[string]interface{}) error {
163+
var rt map[string]any
164+
if err := yaml.Unmarshal(b, &rt); err != nil {
165+
return fmt.Errorf("re-parse: %w", err)
166+
}
167+
for k := range originalTopKeys {
168+
if _, ok := rt[k]; !ok {
169+
return fmt.Errorf("top-level key %q lost during marshal", k)
170+
}
171+
}
172+
got := lookupYamlPath(rt, routesYamlPath, routeName)
173+
if got == nil {
174+
return fmt.Errorf("route %q missing under %q after marshal", routeName, routesYamlPath)
175+
}
176+
if !yamlEqual(got, want) {
177+
return fmt.Errorf("route %q at %q drifted during marshal", routeName, routesYamlPath)
178+
}
179+
return nil
180+
}
181+
182+
func verifyWebhookRouteOnDisk(configPath string, originalTopKeys map[string]struct{}, routesYamlPath, routeName string, want map[string]interface{}) error {
183+
raw, err := os.ReadFile(configPath)
184+
if err != nil {
185+
return fmt.Errorf("read-back: %w", err)
186+
}
187+
return verifyWebhookRouteRoundTrip(raw, originalTopKeys, routesYamlPath, routeName, want)
188+
}
189+
190+
// lookupYamlPath walks routesYamlPath dotted-style and returns the
191+
// entry at the named key, or nil if any intermediate node is missing
192+
// or wrong-shaped. yaml.v3 unmarshals nested maps as
193+
// map[string]interface{} when the top-level target is the same shape,
194+
// so dotted navigation mirrors the JSON walkObject logic.
195+
func lookupYamlPath(obj map[string]any, yamlPath, name string) interface{} {
196+
parts := strings.Split(yamlPath, ".")
197+
cur := obj
198+
for _, p := range parts {
199+
next, ok := cur[p].(map[string]any)
200+
if !ok {
201+
return nil
202+
}
203+
cur = next
204+
}
205+
return cur[name]
206+
}
207+
208+
// ensureWebhookRouteEntry walks (or creates) the dotted path and
209+
// assigns map[routeName] = want. yaml.v3 represents nested maps as
210+
// map[string]any for our object-shaped targets.
211+
func ensureWebhookRouteEntry(obj map[string]any, yamlPath, name string, want map[string]interface{}) error {
212+
parts := strings.Split(yamlPath, ".")
213+
if len(parts) == 0 {
214+
return fmt.Errorf("empty routesYamlPath")
215+
}
216+
cur := obj
217+
for _, p := range parts {
218+
next, ok := cur[p].(map[string]any)
219+
if !ok {
220+
next = map[string]any{}
221+
cur[p] = next
222+
}
223+
cur = next
224+
}
225+
cur[name] = want
226+
return nil
227+
}
228+
229+
// yamlEqual compares two YAML values for deep equality after a
230+
// marshal/unmarshal round-trip on each. Necessary because user-edited
231+
// YAML may have ints stored as int64 / float64 / strings depending on
232+
// the source, while manifest-derived values come from JSON unmarshaling
233+
// where numerics are float64. Round-tripping both through YAML
234+
// normalizes the types.
235+
func yamlEqual(a, b interface{}) bool {
236+
aBytes, err := yaml.Marshal(a)
237+
if err != nil {
238+
return false
239+
}
240+
bBytes, err := yaml.Marshal(b)
241+
if err != nil {
242+
return false
243+
}
244+
var aN, bN interface{}
245+
if err := yaml.Unmarshal(aBytes, &aN); err != nil {
246+
return false
247+
}
248+
if err := yaml.Unmarshal(bBytes, &bN); err != nil {
249+
return false
250+
}
251+
return reflect.DeepEqual(aN, bN)
252+
}

0 commit comments

Comments
 (0)