mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 22:17:36 +00:00
feat(hooks/builtin): embedded registry + UPSERT seed + store-layer readonly
Phase 04 of hooks Wave 1. Adds infrastructure for shipping canonical hook rows with the binary; Phase 05 plants the first real builtin (pii-redactor). - internal/hooks/builtin: loader (//go:embed yaml+js), Seed() with version reconciliation (newer embed overwrites DB; downgrade warn-only; preserves user's enabled toggle), AllowlistFor() for per-id mutable-field lookup. Stable UUIDv5 namespace 082ab084-a25f-52b4-a4a4-eb8a816bd9a8 keys rows across restarts so Seed is idempotent on every boot. - internal/hooks/store.go: ErrBuiltinReadOnly sentinel + WithSeedBypass ctx marker (seeder bypasses the guard; users cannot). - PG + SQLite hook stores: Update/Delete now reject any patch other than enabled=* on source='builtin' rows. Fail-closed on GetByID errors. - Dispatcher: SetBuiltinAllowlistLookup setter lets the gateway wire the registry-backed strict allowlist; unwired tests keep Phase 03 permissive default. Gateway installs a strip-all lookup first so a Load() failure fails closed instead of opening mutation to the permissive default. - i18n: MsgHookBuiltinReadOnly (en/vi/zh). - yaml.v3 promoted to direct dep. - Tests: loader parse + namespace stability + seed idempotency (3 boots → N rows), version bump, downgrade detection, operator BuiltinDisable escape hatch, PG + SQLite readonly guard round-trip.
This commit is contained in:
1 parent
d8e5fad8e4
commit
8ee540dc7e
20 files changed
+981
-5
No files matched your search
@@ -17,6 +17,7 @@ import (
|
||||
"github.com/nextlevelbuilder/goclaw/internal/edition"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/eventbus"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/hooks"
|
||||
hookbuiltin "github.com/nextlevelbuilder/goclaw/internal/hooks/builtin"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/orchestration"
|
||||
httpapi "github.com/nextlevelbuilder/goclaw/internal/http"
|
||||
kg "github.com/nextlevelbuilder/goclaw/internal/knowledgegraph"
|
||||
@@ -148,6 +149,21 @@ func wireExtras(
|
||||
// Agent Hooks (Issue #875) — lifecycle dispatcher + handlers.
|
||||
var hookDispatcher hooks.Dispatcher = hooks.NewNoopDispatcher()
|
||||
if hs, ok := stores.Hooks.(hooks.HookStore); ok && hs != nil {
|
||||
// Phase 04: wire builtin registry. Install a strip-all lookup FIRST so a
|
||||
// Load() failure leaves the dispatcher failing closed (no wide fallback
|
||||
// via the Phase 03 permissive default). On successful Load we swap in the
|
||||
// real per-id allowlist, then UPSERT canonical rows with stable UUIDv5s.
|
||||
// Seed failures log but never block startup.
|
||||
hooks.SetBuiltinAllowlistLookup(func(uuid.UUID) []string { return nil })
|
||||
if err := hookbuiltin.Load(); err != nil {
|
||||
slog.Warn("hooks.builtin_load_failed", "err", err)
|
||||
} else {
|
||||
hooks.SetBuiltinAllowlistLookup(hookbuiltin.AllowlistFor)
|
||||
if err := hookbuiltin.Seed(context.Background(), hs, appCfg.Hooks); err != nil {
|
||||
slog.Warn("hooks.builtin_seed_failed", "err", err)
|
||||
}
|
||||
}
|
||||
|
||||
handlers := buildHookHandlers(stores, providerReg, appCfg.Hooks)
|
||||
stdOpts := hooks.StdDispatcherOpts{
|
||||
Store: hs,
|
||||
|
||||
@@ -43,6 +43,7 @@ require (
|
||||
go.opentelemetry.io/otel/trace v1.40.0
|
||||
golang.org/x/image v0.27.0
|
||||
golang.org/x/time v0.14.0
|
||||
gopkg.in/yaml.v3 v3.0.1
|
||||
modernc.org/sqlite v1.47.0
|
||||
tailscale.com v1.94.2
|
||||
)
|
||||
@@ -156,7 +157,6 @@ require (
|
||||
golang.org/x/term v0.40.0 // indirect
|
||||
golang.zx2c4.com/wintun v0.0.0-20230126152724-0fa3db229ce2 // indirect
|
||||
golang.zx2c4.com/wireguard/windows v0.5.3 // indirect
|
||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||
gvisor.dev/gvisor v0.0.0-20250205023644-9414b50a5633 // indirect
|
||||
modernc.org/libc v1.70.0 // indirect
|
||||
modernc.org/mathutil v1.7.1 // indirect
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
# Builtin Hooks
|
||||
|
||||
Canonical, embed-shipped hook rows. Users may toggle `enabled` but cannot
|
||||
edit content — the embedded `.js` is overwritten on every boot.
|
||||
|
||||
## Adding a new builtin
|
||||
|
||||
1. Pick a stable kebab identifier (`pii-redactor`, `sql-guard`). Never rename:
|
||||
the DB primary key is `UUIDv5(namespace, id + "/" + event)` and renaming
|
||||
would orphan any existing rows.
|
||||
2. Drop `my-feature.js` alongside `builtins.yaml`. The script sees the
|
||||
[goja](https://github.com/dop251/goja) sandboxed globals `event` and
|
||||
`result`, plus the usual `JSON.*`, `Math.*`. No network, no I/O, no timers.
|
||||
3. Add an entry to `builtins.yaml`:
|
||||
|
||||
```yaml
|
||||
- id: my-feature
|
||||
version: 1
|
||||
events: [pre_tool_use]
|
||||
scope: global
|
||||
timeout_ms: 200
|
||||
on_timeout: allow
|
||||
priority: 500
|
||||
mutable_fields: [toolInput.command]
|
||||
source_file: my-feature.js
|
||||
description: "One-line user-facing summary."
|
||||
```
|
||||
|
||||
4. Bump `version` whenever the JS content changes. On next boot, matching
|
||||
rows get UPDATEd in-place (the user's `enabled` toggle is preserved).
|
||||
5. Remove `_placeholder.js` once this is the first real `.js` in the package.
|
||||
|
||||
## Operator escape hatch
|
||||
|
||||
Operators can force-disable any builtin by id via `hooks.builtin_disable` in
|
||||
`config.json` — useful when a builtin misbehaves before the next release:
|
||||
|
||||
```json
|
||||
{
|
||||
"hooks": {
|
||||
"builtin_disable": ["pii-redactor"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The row stays in the DB (history preserved); only `enabled=false` is applied.
|
||||
|
||||
## Version reconciliation rules
|
||||
|
||||
| Embed version | DB version | Outcome |
|
||||
|---------------|------------|------------------------|
|
||||
| missing row | — | INSERT, `enabled=true` |
|
||||
| newer | older | UPDATE content; keep user's `enabled` toggle |
|
||||
| equal | equal | no-op |
|
||||
| older | newer | WARN log; keep DB (no rollback) |
|
||||
|
||||
A downgrade happens when someone rolls back the binary but the DB has already
|
||||
been written by a newer version. We never destructively rewrite; operators
|
||||
must resolve manually.
|
||||
@@ -0,0 +1,2 @@
|
||||
// Placeholder to satisfy `//go:embed *.js` until the first real builtin
|
||||
// ships. Phase 05 adds pii-redactor.js and this file is removed.
|
||||
@@ -0,0 +1,24 @@
|
||||
package builtin
|
||||
|
||||
import "github.com/google/uuid"
|
||||
|
||||
// AllowlistFor returns the `mutable_fields` list for a registered builtin
|
||||
// hook row keyed by the per-event UUID (see BuiltinEventID). Returns nil when
|
||||
// the id is not a registered builtin — caller (dispatcher) treats nil as
|
||||
// "strip all mutations" for defense-in-depth.
|
||||
func AllowlistFor(id uuid.UUID) []string {
|
||||
regMu.RLock()
|
||||
defer regMu.RUnlock()
|
||||
if s, ok := eventIDSpec[id]; ok {
|
||||
return s.MutableFields
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// IsBuiltin reports whether id belongs to a registered builtin row.
|
||||
func IsBuiltin(id uuid.UUID) bool {
|
||||
regMu.RLock()
|
||||
defer regMu.RUnlock()
|
||||
_, ok := eventIDSpec[id]
|
||||
return ok
|
||||
}
|
||||
@@ -0,0 +1,28 @@
|
||||
# GoClaw builtin hooks registry.
|
||||
#
|
||||
# Each entry is shipped with the binary (`//go:embed *.js`) and seeded into
|
||||
# agent_hooks at startup with source='builtin' and a stable UUIDv5. Users may
|
||||
# toggle `enabled` on these rows but cannot edit content — the embedded JS is
|
||||
# the canonical source at every boot.
|
||||
#
|
||||
# Schema:
|
||||
# id kebab-case stable identifier (never rename — breaks UUIDv5)
|
||||
# version integer; bump to force startup overwrite of matching rows
|
||||
# events list of HookEvent values (session_start, user_prompt_submit,
|
||||
# pre_tool_use, post_tool_use, stop, subagent_start,
|
||||
# subagent_stop). One DB row is created per event.
|
||||
# scope global | tenant | agent (builtins default to global)
|
||||
# matcher optional regex applied to tool_name (pre/post_tool_use only)
|
||||
# if_expr optional CEL expression pre-filter
|
||||
# timeout_ms per-hook wall-clock budget
|
||||
# on_timeout allow | block (fail-closed for blocking events)
|
||||
# priority higher = runs earlier in the chain
|
||||
# mutable_fields per-hook allowlist of dotted paths the script may mutate
|
||||
# via updatedInput (e.g. rawInput, toolInput.command)
|
||||
# source_file name of the .js file sitting next to this YAML
|
||||
# description short human summary; surfaced in docs/UI
|
||||
#
|
||||
# Phase 04 ships this registry empty. Phase 05 adds the first exemplar
|
||||
# (pii-redactor). When adding a builtin, also add the .js file + remove
|
||||
# the _placeholder.js once at least one real source_file exists.
|
||||
builtins: []
|
||||
@@ -0,0 +1,130 @@
|
||||
// Package builtin discovers, parses, and seeds builtin hook rows.
|
||||
// Builtins are shipped with the binary via //go:embed and are the only hooks
|
||||
// allowed to mutate event input (source-tier gate in dispatcher).
|
||||
//
|
||||
// Lifecycle: cmd/gateway_managed.go calls Load() once at startup (before
|
||||
// dispatcher wiring), then Seed(ctx, hookStore, cfg.Hooks) after migrations.
|
||||
// Seed is idempotent via stable UUIDv5 IDs derived from the builtin's kebab
|
||||
// name; safe to run on every boot.
|
||||
package builtin
|
||||
|
||||
import (
|
||||
"embed"
|
||||
"fmt"
|
||||
"sync"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
|
||||
// fs carries the registry YAML and one .js per builtin. A `//go:embed *.js`
|
||||
// directive errors at build time if zero .js files exist, so Phase 04 ships a
|
||||
// tiny `_placeholder.js` alongside an empty `builtins:` list. Phase 05 adds
|
||||
// pii-redactor.js and removes the placeholder.
|
||||
//
|
||||
//go:embed *.yaml *.js
|
||||
var fs embed.FS
|
||||
|
||||
// BuiltinNamespace is the stable UUIDv5 namespace for all builtin hook IDs.
|
||||
// Precomputed via uuid.NewSHA1(uuid.NameSpaceDNS, []byte("goclaw.hooks.builtin"))
|
||||
// so every GoClaw build produces the same hash and the seeded rows persist
|
||||
// across restarts / migrations.
|
||||
var BuiltinNamespace = uuid.MustParse("082ab084-a25f-52b4-a4a4-eb8a816bd9a8")
|
||||
|
||||
// BuiltinID returns the stable UUIDv5 derived from a builtin's kebab name.
|
||||
// Used by Seed + dispatcher AllowlistFor lookups.
|
||||
func BuiltinID(name string) uuid.UUID {
|
||||
return uuid.NewSHA1(BuiltinNamespace, []byte(name))
|
||||
}
|
||||
|
||||
// BuiltinEventID returns the stable UUIDv5 for a single (builtin, event)
|
||||
// pair. Each YAML spec lists N events, each materialized as one DB row.
|
||||
func BuiltinEventID(name, event string) uuid.UUID {
|
||||
return uuid.NewSHA1(BuiltinNamespace, []byte(name+"/"+event))
|
||||
}
|
||||
|
||||
// Spec mirrors one entry in builtins.yaml. All fields optional except id,
|
||||
// events, source_file; loader validates at parse time.
|
||||
type Spec struct {
|
||||
ID string `yaml:"id"`
|
||||
Version int `yaml:"version"`
|
||||
Events []string `yaml:"events"`
|
||||
Scope string `yaml:"scope"`
|
||||
Matcher string `yaml:"matcher"`
|
||||
IfExpr string `yaml:"if_expr"`
|
||||
TimeoutMS int `yaml:"timeout_ms"`
|
||||
OnTimeout string `yaml:"on_timeout"`
|
||||
Priority int `yaml:"priority"`
|
||||
MutableFields []string `yaml:"mutable_fields"`
|
||||
SourceFile string `yaml:"source_file"`
|
||||
Description string `yaml:"description"`
|
||||
}
|
||||
|
||||
type catalog struct {
|
||||
Builtins []Spec `yaml:"builtins"`
|
||||
}
|
||||
|
||||
// Registry holds the parsed view of builtins.yaml plus fast allowlist lookup.
|
||||
// Populated once at Load(); read concurrently afterwards under regMu.
|
||||
var (
|
||||
regMu sync.RWMutex
|
||||
specs []Spec // ordered list for deterministic seeding
|
||||
eventIDSpec map[uuid.UUID]*Spec // per-event UUID → owning Spec
|
||||
sourceCache map[string][]byte // source_file → file bytes
|
||||
)
|
||||
|
||||
// Load parses builtins.yaml and caches the registry in memory. Call once at
|
||||
// startup before Seed/AllowlistFor. Idempotent — subsequent calls replace the
|
||||
// cache so tests can reload after fixture edits.
|
||||
func Load() error {
|
||||
raw, err := fs.ReadFile("builtins.yaml")
|
||||
if err != nil {
|
||||
return fmt.Errorf("builtin: read yaml: %w", err)
|
||||
}
|
||||
var cat catalog
|
||||
if err := yaml.Unmarshal(raw, &cat); err != nil {
|
||||
return fmt.Errorf("builtin: parse yaml: %w", err)
|
||||
}
|
||||
|
||||
idx := make(map[uuid.UUID]*Spec, len(cat.Builtins)*2)
|
||||
srcs := make(map[string][]byte, len(cat.Builtins))
|
||||
for i := range cat.Builtins {
|
||||
s := &cat.Builtins[i]
|
||||
if s.ID == "" || s.SourceFile == "" || len(s.Events) == 0 {
|
||||
return fmt.Errorf("builtin: incomplete spec: %+v", *s)
|
||||
}
|
||||
src, err := fs.ReadFile(s.SourceFile)
|
||||
if err != nil {
|
||||
return fmt.Errorf("builtin: missing source file %q: %w", s.SourceFile, err)
|
||||
}
|
||||
srcs[s.SourceFile] = src
|
||||
for _, ev := range s.Events {
|
||||
idx[BuiltinEventID(s.ID, ev)] = s
|
||||
}
|
||||
}
|
||||
|
||||
regMu.Lock()
|
||||
specs = cat.Builtins
|
||||
eventIDSpec = idx
|
||||
sourceCache = srcs
|
||||
regMu.Unlock()
|
||||
return nil
|
||||
}
|
||||
|
||||
// RegisteredSpecs returns a copy of the loaded specs for testing / callers
|
||||
// that need to iterate independently of Load()'s internal state.
|
||||
func RegisteredSpecs() []Spec {
|
||||
regMu.RLock()
|
||||
defer regMu.RUnlock()
|
||||
out := make([]Spec, len(specs))
|
||||
copy(out, specs)
|
||||
return out
|
||||
}
|
||||
|
||||
// source looks up the embedded JS bytes for a SourceFile key.
|
||||
func source(file string) ([]byte, bool) {
|
||||
regMu.RLock()
|
||||
defer regMu.RUnlock()
|
||||
b, ok := sourceCache[file]
|
||||
return b, ok
|
||||
}
|
||||
@@ -0,0 +1,96 @@
|
||||
package builtin
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// TestLoad_EmptyYaml ensures the Phase 04 ship state (empty builtins list +
|
||||
// _placeholder.js) loads without error and leaves the registry empty.
|
||||
func TestLoad_EmptyYaml(t *testing.T) {
|
||||
if err := Load(); err != nil {
|
||||
t.Fatalf("Load: %v", err)
|
||||
}
|
||||
if got := RegisteredSpecs(); len(got) != 0 {
|
||||
t.Errorf("registered=%d, want 0 (empty builtins.yaml)", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuiltinNamespace_Stable guards against a silent namespace change that
|
||||
// would orphan every already-seeded row at the next boot.
|
||||
func TestBuiltinNamespace_Stable(t *testing.T) {
|
||||
want := uuid.MustParse("082ab084-a25f-52b4-a4a4-eb8a816bd9a8")
|
||||
if BuiltinNamespace != want {
|
||||
t.Fatalf("namespace drifted: have %s, want %s", BuiltinNamespace, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuiltinID_Deterministic locks in the per-id hash so that a renamed id
|
||||
// gets caught in review — downgrade-safety depends on stable IDs.
|
||||
func TestBuiltinID_Deterministic(t *testing.T) {
|
||||
a := BuiltinID("pii-redactor")
|
||||
b := BuiltinID("pii-redactor")
|
||||
if a != b {
|
||||
t.Fatalf("BuiltinID non-deterministic: %s vs %s", a, b)
|
||||
}
|
||||
c := BuiltinID("sql-guard")
|
||||
if a == c {
|
||||
t.Fatal("distinct ids produced equal UUID — namespace collision")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuiltinEventID_Distinct ensures each event of a multi-event spec gets
|
||||
// its own row id so per-event enabled toggles don't clobber siblings.
|
||||
func TestBuiltinEventID_Distinct(t *testing.T) {
|
||||
a := BuiltinEventID("pii-redactor", "user_prompt_submit")
|
||||
b := BuiltinEventID("pii-redactor", "pre_tool_use")
|
||||
if a == b {
|
||||
t.Fatal("event UUIDs collided for different events")
|
||||
}
|
||||
if a == BuiltinID("pii-redactor") {
|
||||
t.Fatal("per-event UUID must differ from bare spec id")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAllowlistFor_Unknown returns nil for ids that aren't registered.
|
||||
// Dispatcher treats nil as strip → defense-in-depth against scripts with
|
||||
// forged source='builtin' on a random UUID.
|
||||
func TestAllowlistFor_Unknown(t *testing.T) {
|
||||
if err := Load(); err != nil {
|
||||
t.Fatalf("Load: %v", err)
|
||||
}
|
||||
if got := AllowlistFor(uuid.New()); got != nil {
|
||||
t.Errorf("unknown id → allowlist=%v, want nil", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsBuiltin_Unknown(t *testing.T) {
|
||||
if err := Load(); err != nil {
|
||||
t.Fatalf("Load: %v", err)
|
||||
}
|
||||
if IsBuiltin(uuid.New()) {
|
||||
t.Error("random UUID reported as builtin")
|
||||
}
|
||||
}
|
||||
|
||||
// TestMetadataVersion covers the JSONB float64 decode path plus int fast path.
|
||||
func TestMetadataVersion(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
meta map[string]any
|
||||
want int
|
||||
}{
|
||||
{"nil", nil, 0},
|
||||
{"missing", map[string]any{}, 0},
|
||||
{"int", map[string]any{"version": 3}, 3},
|
||||
{"int64", map[string]any{"version": int64(4)}, 4},
|
||||
{"float64", map[string]any{"version": float64(5)}, 5},
|
||||
{"string-ignored", map[string]any{"version": "oops"}, 0},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := metadataVersion(tc.meta); got != tc.want {
|
||||
t.Errorf("%s: got %d want %d", tc.name, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,160 @@
|
||||
package builtin
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/config"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/hooks"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
)
|
||||
|
||||
// Seed UPSERTs every registered builtin into hookStore following the
|
||||
// version-reconciliation rules in phase-04:
|
||||
//
|
||||
// - DB row missing → INSERT one row per event with source='builtin'.
|
||||
// - DB version < embed version → UPDATE content (preserve enabled toggle).
|
||||
// - DB version == embed version → no-op.
|
||||
// - DB version > embed version → WARN + keep DB (no rollback).
|
||||
//
|
||||
// Runs AFTER migrations and BEFORE dispatcher handler registration. Uses
|
||||
// store.RoleOwner + hooks.WithSeedBypass to traverse the tenant-scope guard
|
||||
// and the builtin-readonly Update protection; both markers are process-local.
|
||||
func Seed(ctx context.Context, hookStore hooks.HookStore, cfg config.HooksConfig) error {
|
||||
regMu.RLock()
|
||||
all := make([]Spec, len(specs))
|
||||
copy(all, specs)
|
||||
regMu.RUnlock()
|
||||
|
||||
disabled := make(map[string]struct{}, len(cfg.BuiltinDisable))
|
||||
for _, d := range cfg.BuiltinDisable {
|
||||
disabled[d] = struct{}{}
|
||||
}
|
||||
|
||||
ctx = store.WithRole(ctx, store.RoleOwner)
|
||||
ctx = hooks.WithSeedBypass(ctx)
|
||||
|
||||
for i := range all {
|
||||
s := all[i]
|
||||
_, forceDisabled := disabled[s.ID]
|
||||
if err := seedOne(ctx, hookStore, s, forceDisabled); err != nil {
|
||||
slog.Warn("hooks.builtin_seed_failed", "id", s.ID, "err", err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// seedOne reconciles one spec (N events → N rows) against the store.
|
||||
func seedOne(ctx context.Context, hs hooks.HookStore, s Spec, forceDisabled bool) error {
|
||||
src, ok := source(s.SourceFile)
|
||||
if !ok {
|
||||
return fmt.Errorf("source not cached: %s", s.SourceFile)
|
||||
}
|
||||
|
||||
cfgMap := map[string]any{"source": string(src)}
|
||||
metadata := map[string]any{
|
||||
"builtin": true,
|
||||
"version": s.Version,
|
||||
"description": s.Description,
|
||||
}
|
||||
|
||||
for _, ev := range s.Events {
|
||||
id := BuiltinEventID(s.ID, ev)
|
||||
if err := reconcile(ctx, hs, id, s, ev, cfgMap, metadata, forceDisabled); err != nil {
|
||||
return fmt.Errorf("%s/%s: %w", s.ID, ev, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// reconcile handles insert/update/noop for a single builtin-event row.
|
||||
func reconcile(
|
||||
ctx context.Context,
|
||||
hs hooks.HookStore,
|
||||
id uuid.UUID,
|
||||
s Spec,
|
||||
event string,
|
||||
cfgMap, metadata map[string]any,
|
||||
forceDisabled bool,
|
||||
) error {
|
||||
current, err := hs.GetByID(ctx, id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("get: %w", err)
|
||||
}
|
||||
|
||||
if current == nil {
|
||||
cfg := hooks.HookConfig{
|
||||
ID: id,
|
||||
TenantID: hooks.SentinelTenantID,
|
||||
Event: hooks.HookEvent(event),
|
||||
HandlerType: hooks.HandlerScript,
|
||||
Scope: hooks.Scope(s.Scope),
|
||||
Config: cfgMap,
|
||||
Matcher: s.Matcher,
|
||||
IfExpr: s.IfExpr,
|
||||
TimeoutMS: s.TimeoutMS,
|
||||
OnTimeout: hooks.Decision(s.OnTimeout),
|
||||
Priority: s.Priority,
|
||||
Enabled: !forceDisabled,
|
||||
Source: hooks.SourceBuiltin,
|
||||
Metadata: metadata,
|
||||
}
|
||||
if _, err := hs.Create(ctx, cfg); err != nil {
|
||||
return fmt.Errorf("create: %w", err)
|
||||
}
|
||||
slog.Info("hooks.builtin_seeded", "id", s.ID, "event", event, "version", s.Version)
|
||||
return nil
|
||||
}
|
||||
|
||||
dbVersion := metadataVersion(current.Metadata)
|
||||
switch {
|
||||
case s.Version > dbVersion:
|
||||
patch := map[string]any{
|
||||
"config": cfgMap,
|
||||
"matcher": s.Matcher,
|
||||
"if_expr": s.IfExpr,
|
||||
"timeout_ms": s.TimeoutMS,
|
||||
"on_timeout": string(hooks.Decision(s.OnTimeout)),
|
||||
"priority": s.Priority,
|
||||
"metadata": metadata,
|
||||
}
|
||||
if err := hs.Update(ctx, id, patch); err != nil {
|
||||
return fmt.Errorf("update: %w", err)
|
||||
}
|
||||
slog.Info("hooks.builtin_updated", "id", s.ID, "event", event,
|
||||
"from_version", dbVersion, "to_version", s.Version)
|
||||
case s.Version < dbVersion:
|
||||
slog.Warn("hooks.builtin_downgrade_detected",
|
||||
"id", s.ID, "event", event,
|
||||
"db_version", dbVersion, "embed_version", s.Version,
|
||||
"note", "DB content kept; no rollback performed")
|
||||
}
|
||||
|
||||
if forceDisabled && current.Enabled {
|
||||
if err := hs.Update(ctx, id, map[string]any{"enabled": false}); err != nil {
|
||||
return fmt.Errorf("force-disable: %w", err)
|
||||
}
|
||||
slog.Info("hooks.builtin_disabled_by_config", "id", s.ID, "event", event)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// metadataVersion reads metadata["version"] tolerating JSONB float64 decoding.
|
||||
func metadataVersion(m map[string]any) int {
|
||||
if m == nil {
|
||||
return 0
|
||||
}
|
||||
switch v := m["version"].(type) {
|
||||
case int:
|
||||
return v
|
||||
case int64:
|
||||
return int(v)
|
||||
case float64:
|
||||
return int(v)
|
||||
default:
|
||||
return 0
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,255 @@
|
||||
package builtin
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/config"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/hooks"
|
||||
)
|
||||
|
||||
// fakeHookStore is an in-memory HookStore just rich enough to test seed
|
||||
// reconciliation. Implements only the methods the seeder touches (Create,
|
||||
// GetByID, Update); others return zero values / nil.
|
||||
type fakeHookStore struct {
|
||||
rows map[uuid.UUID]*hooks.HookConfig
|
||||
}
|
||||
|
||||
func newFakeHookStore() *fakeHookStore {
|
||||
return &fakeHookStore{rows: map[uuid.UUID]*hooks.HookConfig{}}
|
||||
}
|
||||
|
||||
func (f *fakeHookStore) Create(_ context.Context, cfg hooks.HookConfig) (uuid.UUID, error) {
|
||||
if cfg.ID == uuid.Nil {
|
||||
cfg.ID = uuid.Must(uuid.NewV7())
|
||||
}
|
||||
if _, exists := f.rows[cfg.ID]; exists {
|
||||
// Mimic DB primary-key conflict to catch non-idempotent seeds.
|
||||
return uuid.Nil, errDuplicate
|
||||
}
|
||||
c := cfg
|
||||
f.rows[cfg.ID] = &c
|
||||
return cfg.ID, nil
|
||||
}
|
||||
|
||||
func (f *fakeHookStore) GetByID(_ context.Context, id uuid.UUID) (*hooks.HookConfig, error) {
|
||||
if r, ok := f.rows[id]; ok {
|
||||
c := *r
|
||||
return &c, nil
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (f *fakeHookStore) List(context.Context, hooks.ListFilter) ([]hooks.HookConfig, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (f *fakeHookStore) Update(_ context.Context, id uuid.UUID, updates map[string]any) error {
|
||||
r, ok := f.rows[id]
|
||||
if !ok {
|
||||
return errNotFound
|
||||
}
|
||||
for k, v := range updates {
|
||||
switch k {
|
||||
case "enabled":
|
||||
if b, ok := v.(bool); ok {
|
||||
r.Enabled = b
|
||||
}
|
||||
case "metadata":
|
||||
if m, ok := v.(map[string]any); ok {
|
||||
r.Metadata = m
|
||||
}
|
||||
case "config":
|
||||
if m, ok := v.(map[string]any); ok {
|
||||
r.Config = m
|
||||
}
|
||||
case "matcher":
|
||||
if s, ok := v.(string); ok {
|
||||
r.Matcher = s
|
||||
}
|
||||
case "if_expr":
|
||||
if s, ok := v.(string); ok {
|
||||
r.IfExpr = s
|
||||
}
|
||||
case "priority":
|
||||
if n, ok := v.(int); ok {
|
||||
r.Priority = n
|
||||
}
|
||||
case "timeout_ms":
|
||||
if n, ok := v.(int); ok {
|
||||
r.TimeoutMS = n
|
||||
}
|
||||
}
|
||||
}
|
||||
r.Version++
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeHookStore) Delete(_ context.Context, id uuid.UUID) error {
|
||||
delete(f.rows, id)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeHookStore) ResolveForEvent(context.Context, hooks.Event) ([]hooks.HookConfig, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (f *fakeHookStore) WriteExecution(context.Context, hooks.HookExecution) error { return nil }
|
||||
|
||||
type errStr string
|
||||
|
||||
func (e errStr) Error() string { return string(e) }
|
||||
|
||||
const (
|
||||
errDuplicate errStr = "fake: duplicate key"
|
||||
errNotFound errStr = "fake: not found"
|
||||
)
|
||||
|
||||
// withTestRegistry installs a one-spec registry for the test and restores the
|
||||
// prior state on cleanup. Serial access only — tests must not run in parallel.
|
||||
func withTestRegistry(t *testing.T, s Spec, src []byte) {
|
||||
t.Helper()
|
||||
regMu.Lock()
|
||||
prevSpecs := specs
|
||||
prevIdx := eventIDSpec
|
||||
prevSrc := sourceCache
|
||||
specs = []Spec{s}
|
||||
eventIDSpec = map[uuid.UUID]*Spec{}
|
||||
for _, ev := range s.Events {
|
||||
eventIDSpec[BuiltinEventID(s.ID, ev)] = &specs[0]
|
||||
}
|
||||
sourceCache = map[string][]byte{s.SourceFile: src}
|
||||
regMu.Unlock()
|
||||
t.Cleanup(func() {
|
||||
regMu.Lock()
|
||||
specs = prevSpecs
|
||||
eventIDSpec = prevIdx
|
||||
sourceCache = prevSrc
|
||||
regMu.Unlock()
|
||||
})
|
||||
}
|
||||
|
||||
func fixtureSpec(version int) Spec {
|
||||
return Spec{
|
||||
ID: "test-redactor",
|
||||
Version: version,
|
||||
Events: []string{"user_prompt_submit", "pre_tool_use"},
|
||||
Scope: "global",
|
||||
TimeoutMS: 500,
|
||||
OnTimeout: "allow",
|
||||
Priority: 900,
|
||||
MutableFields: []string{"rawInput"},
|
||||
SourceFile: "test-redactor.js",
|
||||
Description: "test",
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeed_InsertsOneRowPerEvent verifies seed creates N rows for a spec with
|
||||
// N events using stable UUIDv5s — this is the H9 idempotency check.
|
||||
func TestSeed_InsertsOneRowPerEvent(t *testing.T) {
|
||||
spec := fixtureSpec(1)
|
||||
withTestRegistry(t, spec, []byte("// v1"))
|
||||
|
||||
fs := newFakeHookStore()
|
||||
if err := Seed(context.Background(), fs, config.HooksConfig{}); err != nil {
|
||||
t.Fatalf("Seed: %v", err)
|
||||
}
|
||||
if got := len(fs.rows); got != len(spec.Events) {
|
||||
t.Fatalf("rows=%d, want %d", got, len(spec.Events))
|
||||
}
|
||||
for _, ev := range spec.Events {
|
||||
id := BuiltinEventID(spec.ID, ev)
|
||||
row, ok := fs.rows[id]
|
||||
if !ok {
|
||||
t.Fatalf("missing row for event %q", ev)
|
||||
}
|
||||
if row.Source != hooks.SourceBuiltin {
|
||||
t.Errorf("source=%q, want builtin", row.Source)
|
||||
}
|
||||
if !row.Enabled {
|
||||
t.Error("row disabled; expected enabled=true on fresh seed")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeed_Idempotent validates H9 — re-running seed against the same registry
|
||||
// creates zero additional rows. A regression here would manifest as duplicate
|
||||
// rows every boot once the unique index is dropped by Phase 03.
|
||||
func TestSeed_Idempotent(t *testing.T) {
|
||||
spec := fixtureSpec(1)
|
||||
withTestRegistry(t, spec, []byte("// v1"))
|
||||
|
||||
fs := newFakeHookStore()
|
||||
for i := range 3 {
|
||||
if err := Seed(context.Background(), fs, config.HooksConfig{}); err != nil {
|
||||
t.Fatalf("Seed iter=%d: %v", i, err)
|
||||
}
|
||||
}
|
||||
if got := len(fs.rows); got != len(spec.Events) {
|
||||
t.Fatalf("after 3 seeds rows=%d, want %d (duplicates!)", got, len(spec.Events))
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeed_VersionBump updates existing rows when the embed version grows.
|
||||
// Preserves the user's enabled toggle.
|
||||
func TestSeed_VersionBump(t *testing.T) {
|
||||
spec := fixtureSpec(1)
|
||||
withTestRegistry(t, spec, []byte("// v1"))
|
||||
fs := newFakeHookStore()
|
||||
if err := Seed(context.Background(), fs, config.HooksConfig{}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Simulate user disabling a row.
|
||||
id := BuiltinEventID(spec.ID, "pre_tool_use")
|
||||
fs.rows[id].Enabled = false
|
||||
|
||||
// Replace registry with v2.
|
||||
withTestRegistry(t, fixtureSpec(2), []byte("// v2"))
|
||||
if err := Seed(context.Background(), fs, config.HooksConfig{}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if ver := fs.rows[id].Metadata["version"]; ver != 2 {
|
||||
t.Errorf("version not bumped: %v", ver)
|
||||
}
|
||||
if fs.rows[id].Enabled {
|
||||
t.Error("seed overwrote user's disabled toggle")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeed_DowngradeDetected keeps the DB row intact when the embed version
|
||||
// is older than the DB version (rollback scenario).
|
||||
func TestSeed_DowngradeDetected(t *testing.T) {
|
||||
withTestRegistry(t, fixtureSpec(3), []byte("// v3"))
|
||||
fs := newFakeHookStore()
|
||||
if err := Seed(context.Background(), fs, config.HooksConfig{}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
id := BuiltinEventID("test-redactor", "user_prompt_submit")
|
||||
gotV := fs.rows[id].Metadata["version"]
|
||||
|
||||
// Now simulate rollback to v2 embed.
|
||||
withTestRegistry(t, fixtureSpec(2), []byte("// v2"))
|
||||
if err := Seed(context.Background(), fs, config.HooksConfig{}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if gotV2 := fs.rows[id].Metadata["version"]; gotV2 != gotV {
|
||||
t.Errorf("downgrade mutated row: %v → %v", gotV, gotV2)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeed_BuiltinDisableForcesOff applies the operator escape-hatch list.
|
||||
func TestSeed_BuiltinDisableForcesOff(t *testing.T) {
|
||||
withTestRegistry(t, fixtureSpec(1), []byte("// v1"))
|
||||
fs := newFakeHookStore()
|
||||
cfg := config.HooksConfig{BuiltinDisable: []string{"test-redactor"}}
|
||||
if err := Seed(context.Background(), fs, cfg); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for id, row := range fs.rows {
|
||||
if row.Enabled {
|
||||
t.Errorf("row %s enabled despite builtin_disable", id)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -318,10 +318,27 @@ func applyBuiltinMutation(ev *Event, updated map[string]any, allowlist []string)
|
||||
}
|
||||
}
|
||||
|
||||
// builtinAllowlistFor returns the field allowlist for a builtin hook.
|
||||
// Phase 03 ships a permissive default; Phase 04 overrides with a per-hook
|
||||
// allowlist loaded from builtins.yaml at startup.
|
||||
func builtinAllowlistFor(_ uuid.UUID) []string {
|
||||
// builtinAllowlistLookup is set at startup by the builtin package wiring
|
||||
// (cmd/gateway_managed.go calls SetBuiltinAllowlistLookup(builtin.AllowlistFor)).
|
||||
// When nil (tests that don't wire the registry) we fall back to the Phase 03
|
||||
// permissive default so legacy mutation tests stay green.
|
||||
var builtinAllowlistLookup func(id uuid.UUID) []string
|
||||
|
||||
// SetBuiltinAllowlistLookup installs the per-id allowlist function. Called
|
||||
// once at startup from cmd/gateway_managed.go. Safe to leave unset in tests —
|
||||
// callers get the permissive default (rawInput + toolInput) below.
|
||||
func SetBuiltinAllowlistLookup(f func(uuid.UUID) []string) {
|
||||
builtinAllowlistLookup = f
|
||||
}
|
||||
|
||||
// builtinAllowlistFor returns the field allowlist for a builtin hook row.
|
||||
// Registered builtin → YAML mutable_fields. Unknown id under a wired lookup
|
||||
// → empty allowlist (mutations stripped, defense-in-depth). Unset lookup
|
||||
// (unit tests) → Phase 03 permissive default.
|
||||
func builtinAllowlistFor(id uuid.UUID) []string {
|
||||
if builtinAllowlistLookup != nil {
|
||||
return builtinAllowlistLookup(id)
|
||||
}
|
||||
return []string{"rawInput", "toolInput"}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,10 +2,36 @@ package hooks
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// ErrBuiltinReadOnly is returned by Update/Delete when a caller attempts to
|
||||
// modify a row with source='builtin' beyond toggling `enabled`. Builtin rows
|
||||
// are canonical — content comes from embedded JS at every startup (Phase 04
|
||||
// seed). UI should render them read-only except for the enabled toggle.
|
||||
// Message is surfaced via i18n key hook.builtin_readonly.
|
||||
var ErrBuiltinReadOnly = errors.New("hook: builtin row is read-only except enabled toggle")
|
||||
|
||||
// seedBypassKey is the private context key used by Phase 04 builtin seeder to
|
||||
// bypass the Update/Delete builtin-readonly guard. Process-private: only the
|
||||
// seeder sets it. Ensures user-facing writes still fail ErrBuiltinReadOnly.
|
||||
type seedBypassKey struct{}
|
||||
|
||||
// WithSeedBypass marks ctx as originating from the builtin seeder. Store
|
||||
// implementations MUST consult IsSeedBypass(ctx) before rejecting writes on
|
||||
// builtin rows. Any other caller is denied.
|
||||
func WithSeedBypass(ctx context.Context) context.Context {
|
||||
return context.WithValue(ctx, seedBypassKey{}, true)
|
||||
}
|
||||
|
||||
// IsSeedBypass reports whether ctx carries the builtin-seeder bypass marker.
|
||||
func IsSeedBypass(ctx context.Context) bool {
|
||||
v, _ := ctx.Value(seedBypassKey{}).(bool)
|
||||
return v
|
||||
}
|
||||
|
||||
// ListFilter narrows results from HookStore.List.
|
||||
type ListFilter struct {
|
||||
TenantID *uuid.UUID
|
||||
|
||||
@@ -210,5 +210,6 @@ func init() {
|
||||
MsgHookCircuitBreakerTripped: "hook auto-disabled after repeated failures",
|
||||
MsgHookBudgetExceeded: "tenant hook token budget exceeded",
|
||||
MsgHookPerTurnCapReached: "hook invocation per-turn cap reached",
|
||||
MsgHookBuiltinReadOnly: "builtin hooks are read-only except for the enabled toggle",
|
||||
})
|
||||
}
|
||||
@@ -210,5 +210,6 @@ func init() {
|
||||
MsgHookCircuitBreakerTripped: "hook đã tự tắt sau nhiều lần thất bại liên tiếp",
|
||||
MsgHookBudgetExceeded: "tenant đã vượt ngân sách token cho hook",
|
||||
MsgHookPerTurnCapReached: "đã đạt giới hạn số lần gọi hook trong một lượt",
|
||||
MsgHookBuiltinReadOnly: "hook dựng sẵn chỉ cho phép bật/tắt, không thể chỉnh sửa",
|
||||
})
|
||||
}
|
||||
@@ -210,5 +210,6 @@ func init() {
|
||||
MsgHookCircuitBreakerTripped: "钩子在多次失败后已自动禁用",
|
||||
MsgHookBudgetExceeded: "租户钩子令牌预算已耗尽",
|
||||
MsgHookPerTurnCapReached: "单轮钩子调用次数已达上限",
|
||||
MsgHookBuiltinReadOnly: "内置钩子只读,仅允许切换启用状态",
|
||||
})
|
||||
}
|
||||
@@ -214,4 +214,5 @@ const (
|
||||
MsgHookCircuitBreakerTripped = "hook.circuit_breaker_tripped" // "hook auto-disabled after repeated failures"
|
||||
MsgHookBudgetExceeded = "hook.budget_exceeded" // "tenant hook token budget exceeded"
|
||||
MsgHookPerTurnCapReached = "hook.per_turn_cap_reached" // "hook invocation per-turn cap reached"
|
||||
MsgHookBuiltinReadOnly = "hook.builtin_readonly" // "builtin hooks are read-only except for the enabled toggle"
|
||||
)
|
||||
@@ -199,6 +199,25 @@ func (s *PGHookStore) Update(ctx context.Context, id uuid.UUID, updates map[stri
|
||||
return fmt.Errorf("callers must not include 'version' in update map")
|
||||
}
|
||||
|
||||
// Builtin-row protection (Phase 04). Rows with source='builtin' are canonical
|
||||
// embed-seeded artifacts: UI/user writes may only toggle `enabled`. The
|
||||
// builtin seeder itself passes through via hooks.WithSeedBypass ctx.
|
||||
// Fail-closed: a transient GetByID error must not let a non-enabled patch
|
||||
// slip through against a builtin row.
|
||||
if !hooks.IsSeedBypass(ctx) {
|
||||
current, err := s.GetByID(ctx, id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("builtin guard: %w", err)
|
||||
}
|
||||
if current != nil && current.Source == hooks.SourceBuiltin {
|
||||
for k := range updates {
|
||||
if k != "enabled" {
|
||||
return hooks.ErrBuiltinReadOnly
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Marshal map/slice values to JSON for JSONB columns.
|
||||
for k, v := range updates {
|
||||
switch k {
|
||||
@@ -256,6 +275,18 @@ func (s *PGHookStore) Update(ctx context.Context, id uuid.UUID, updates map[stri
|
||||
// ─── Delete ──────────────────────────────────────────────────────────────────
|
||||
|
||||
func (s *PGHookStore) Delete(ctx context.Context, id uuid.UUID) error {
|
||||
// Builtin-row protection (Phase 04): reject unless caller marked seed-bypass.
|
||||
// Fail-closed on GetByID errors so we don't accidentally delete a builtin.
|
||||
if !hooks.IsSeedBypass(ctx) {
|
||||
current, err := s.GetByID(ctx, id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("builtin guard: %w", err)
|
||||
}
|
||||
if current != nil && current.Source == hooks.SourceBuiltin {
|
||||
return hooks.ErrBuiltinReadOnly
|
||||
}
|
||||
}
|
||||
|
||||
q := "DELETE FROM agent_hooks WHERE id = $1"
|
||||
args := []any{id}
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ package pg
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"os"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -421,3 +422,52 @@ func TestPGHookStore_CreateHonorsFixedID(t *testing.T) {
|
||||
t.Fatal("Create returned nil id for cfg.ID=uuid.Nil path")
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Phase 04: builtin-row readonly protection ───────────────────────────────
|
||||
|
||||
// TestPGHookStore_BuiltinReadOnly exercises the Phase 04 guard: user-facing
|
||||
// writes on a source='builtin' row may only toggle enabled; every other patch
|
||||
// must surface ErrBuiltinReadOnly. The seed bypass marker unlocks full writes
|
||||
// for the loader package only.
|
||||
func TestPGHookStore_BuiltinReadOnly(t *testing.T) {
|
||||
db := hooksTestDB(t)
|
||||
tenantID, _ := seedTenantAndAgent(t, db)
|
||||
s := NewPGHookStore(db)
|
||||
|
||||
// Seed a builtin row via WithSeedBypass (the only authorized path).
|
||||
seedCtx := hooks.WithSeedBypass(store.WithRole(masterCtx(), store.RoleOwner))
|
||||
cfg := minimalHook(hooks.SentinelTenantID, hooks.EventUserPromptSubmit)
|
||||
cfg.Source = hooks.SourceBuiltin
|
||||
cfg.Scope = hooks.ScopeGlobal
|
||||
cfg.HandlerType = hooks.HandlerScript
|
||||
cfg.Config = map[string]any{"source": "// v1"}
|
||||
fixedID := uuid.MustParse("11111111-2222-3333-4444-555555555555")
|
||||
cfg.ID = fixedID
|
||||
id, err := s.Create(seedCtx, cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("seed create: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { s.Delete(seedCtx, id) })
|
||||
|
||||
// User ctx: tenant-scoped. Non-enabled patch must be rejected.
|
||||
userCtx := tenantScopedCtx(tenantID)
|
||||
err = s.Update(userCtx, id, map[string]any{"matcher": "evil"})
|
||||
if !errors.Is(err, hooks.ErrBuiltinReadOnly) {
|
||||
t.Fatalf("Update(matcher) err=%v, want ErrBuiltinReadOnly", err)
|
||||
}
|
||||
|
||||
// Enabled toggle through master context is allowed.
|
||||
if err := s.Update(masterCtx(), id, map[string]any{"enabled": false}); err != nil {
|
||||
t.Fatalf("Update(enabled) should succeed on builtin: %v", err)
|
||||
}
|
||||
|
||||
// Delete blocked for users.
|
||||
if err := s.Delete(userCtx, id); !errors.Is(err, hooks.ErrBuiltinReadOnly) {
|
||||
t.Fatalf("Delete user err=%v, want ErrBuiltinReadOnly", err)
|
||||
}
|
||||
|
||||
// Seed bypass unlocks full writes (round-trip to prove).
|
||||
if err := s.Update(seedCtx, id, map[string]any{"matcher": "ok"}); err != nil {
|
||||
t.Fatalf("seed-bypass Update should succeed: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -205,6 +205,24 @@ func (s *SqliteHookStore) Update(ctx context.Context, id uuid.UUID, updates map[
|
||||
return fmt.Errorf("callers must not include 'version' in update map")
|
||||
}
|
||||
|
||||
// Builtin-row protection (Phase 04). User/UI writes may only toggle
|
||||
// `enabled`; the builtin seeder itself bypasses this via hooks.WithSeedBypass.
|
||||
// Fail-closed on GetByID errors — silently passing here could let wide
|
||||
// patches through on a transient DB read failure.
|
||||
if !hooks.IsSeedBypass(ctx) {
|
||||
current, err := s.GetByID(ctx, id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("builtin guard: %w", err)
|
||||
}
|
||||
if current != nil && current.Source == hooks.SourceBuiltin {
|
||||
for k := range updates {
|
||||
if k != "enabled" {
|
||||
return hooks.ErrBuiltinReadOnly
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Marshal map/slice values to JSON strings for SQLite TEXT columns.
|
||||
for k, v := range updates {
|
||||
switch k {
|
||||
@@ -258,6 +276,18 @@ func (s *SqliteHookStore) Update(ctx context.Context, id uuid.UUID, updates map[
|
||||
// ─── Delete ──────────────────────────────────────────────────────────────────
|
||||
|
||||
func (s *SqliteHookStore) Delete(ctx context.Context, id uuid.UUID) error {
|
||||
// Builtin-row protection (Phase 04): reject unless caller marked seed-bypass.
|
||||
// Fail-closed on GetByID errors.
|
||||
if !hooks.IsSeedBypass(ctx) {
|
||||
current, err := s.GetByID(ctx, id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("builtin guard: %w", err)
|
||||
}
|
||||
if current != nil && current.Source == hooks.SourceBuiltin {
|
||||
return hooks.ErrBuiltinReadOnly
|
||||
}
|
||||
}
|
||||
|
||||
q := "DELETE FROM agent_hooks WHERE id = ?"
|
||||
args := []any{id.String()}
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ package sqlitestore
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -438,3 +439,50 @@ func TestSQLiteHookStore_GlobalScopeVisibleToTenant(t *testing.T) {
|
||||
t.Error("global hook not visible in ResolveForEvent from tenant scope")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSQLiteHookStore_BuiltinReadOnly mirrors the PG test: user-facing writes
|
||||
// on source='builtin' rows may only toggle enabled; WithSeedBypass unlocks.
|
||||
func TestSQLiteHookStore_BuiltinReadOnly(t *testing.T) {
|
||||
db := newHookTestDB(t)
|
||||
tenantID, _ := seedHookTenantAgent(t, db)
|
||||
s := NewSQLiteHookStore(db)
|
||||
|
||||
seedCtx := hooks.WithSeedBypass(store.WithRole(sqliteMasterCtx(), store.RoleOwner))
|
||||
|
||||
cfg := hooks.HookConfig{
|
||||
ID: uuid.MustParse("11111111-2222-3333-4444-555555555555"),
|
||||
TenantID: hooks.SentinelTenantID,
|
||||
Event: hooks.EventUserPromptSubmit,
|
||||
HandlerType: hooks.HandlerScript,
|
||||
Scope: hooks.ScopeGlobal,
|
||||
Config: map[string]any{"source": "// v1"},
|
||||
Metadata: map[string]any{"builtin": true, "version": 1},
|
||||
TimeoutMS: 500,
|
||||
OnTimeout: hooks.DecisionAllow,
|
||||
Source: hooks.SourceBuiltin,
|
||||
Enabled: true,
|
||||
}
|
||||
id, err := s.Create(seedCtx, cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("seed create: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { s.Delete(seedCtx, id) })
|
||||
|
||||
userCtx := sqliteTenantCtx(tenantID)
|
||||
|
||||
if err := s.Update(userCtx, id, map[string]any{"matcher": "evil"}); !errors.Is(err, hooks.ErrBuiltinReadOnly) {
|
||||
t.Fatalf("Update(matcher) err=%v, want ErrBuiltinReadOnly", err)
|
||||
}
|
||||
|
||||
if err := s.Update(sqliteMasterCtx(), id, map[string]any{"enabled": false}); err != nil {
|
||||
t.Fatalf("Update(enabled) should succeed: %v", err)
|
||||
}
|
||||
|
||||
if err := s.Delete(userCtx, id); !errors.Is(err, hooks.ErrBuiltinReadOnly) {
|
||||
t.Fatalf("Delete user err=%v, want ErrBuiltinReadOnly", err)
|
||||
}
|
||||
|
||||
if err := s.Update(seedCtx, id, map[string]any{"matcher": "ok"}); err != nil {
|
||||
t.Fatalf("seed-bypass Update should succeed: %v", err)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user