fae2303db3
* feat(plugins): per-plugin settings pages driven by manifest config_schema Settings > Plugins rows now open /settings/plugins/:id — a detail page showing the manifest plus a settings form generated from the plugin author's config_schema. Secret fields (secret: true / format: password) are masked on the operator API and kept on masked re-submits; a running plugin restarts on save and reads its config (cleartext secrets included) through the new Host GetConfig gRPC RPC. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): write plugin config file owner-only (0600) Operator config can now carry cleartext secret values (config_schema fields marked secret), so <id>.config.yml must not be world-readable — match the 0600 mode the record file already uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(plugins): vault-backed plugin secrets — Get/Set/DeleteSecret RPCs and encrypted config secrets Plugins get vault-backed secret primitives on the Host API (GetSecret/SetSecret/DeleteSecret, capability-gated, namespaced to plugin:<id>㊙️<key> so a plugin can only touch its own entries; purged on uninstall). Secret config_schema fields now live in kandev's AES-256-GCM vault too — <id>.config.yml persists only a vault reference, and the Host GetConfig RPC resolves it back to cleartext for the plugin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(plugins): serve PR screenshots from the media/pr-1761-screenshots ref instead of the repo The PR body now points at raw URLs pinned to an unmerged media ref, so the binaries never land in main (PRs are squash-merged; only the branch tip tree merges). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): address review feedback on settings validation and save flow - integer form fields reject non-integral input instead of parseInt-truncating - enum selections keep their schema-declared types (numeric enums round-trip) - backend enum validation normalizes int-vs-float64 before comparison - maskSecrets masks any non-zero secret value regardless of runtime type - save flow splits PATCH from refetch: a refetch failure no longer reports a successful save as an error, and typed secrets are masked on screen - optional enum fields gain a 'Not set' option so they can be cleared - resolveConfigSecrets copies before mutating; restart-failure SetStatus errors are logged; SDK test fake returns a non-nil empty config map - add missing tests: usePluginConfigForm hook, getPluginConfig client, integer/enum regression cases Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): harden secret config invariants from review - reject non-string values on declared secret fields at validation time, closing the path where a numeric/boolean secret bypassed the vault and persisted cleartext in the config file - delete removed secret fields' vault entries only AFTER the config commit succeeds, so a failed SetConfig never leaves the still-current config referencing a deleted vault entry - uninstall fails closed: the plugin's vault namespace is purged before any destructive step, and a cleanup failure aborts the uninstall (retryable) instead of letting a reinstall inherit stale secrets Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): failure-atomic config secrets, quiesced uninstall purge, ctx propagation - UpdateConfig is now failure-atomic for secret overwrites: storeConfigSecrets snapshots each vault entry before writing and returns a rollback that restores prior values (or deletes newly-created ones). A vault.Set failure mid-loop or a failed SetConfig commit rolls back, so a request reported as failed never changes the value a still-current config ref resolves to. - Uninstall stops the plugin process BEFORE purging its vault namespace, so the plugin can no longer race the cleanup with a SetSecret between ListIDs and the deletes; the purge stays fail-closed and retryable. - Thread the request context through Uninstall -> deletePluginSecrets instead of context.Background(), per the pass-context-through-chains convention. - Consolidate the 'secret not found:' prefix check into isSecretNotFound (host.go + service rollback share one definition). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): detach rollback context, fail closed without a vault, reconcile uninstall status - vault rollback closures now run on context.WithoutCancel(ctx): a request cancelled mid-save (browser closed) can no longer abort the rollback and leave the vault holding new values while the unchanged config still points at them (Greptile P1). - storeConfigSecrets fails closed (errSecretVaultRequired) when a plugin declares secret config fields but no vault is wired, instead of silently persisting the secret in cleartext. - vaultRestoreFunc distinguishes not-found from a genuine Reveal error: a backend read failure aborts the update before writing rather than risk a rollback that deletes a real secret; rollback now returns errors, joined into UpdateConfig's failure so an inconsistent vault is surfaced, not just logged. - Uninstall reconciles lifecycle status when a secret-purge failure aborts after the process was stopped: a previously-running plugin transitions to error and notifies observers instead of being reported active with no process running. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): detach removed-secret cleanup ctx and clear stale save dirty flag - cleanupRemovedConfigSecrets now runs on context.WithoutCancel(ctx), matching the rollback path, so a client disconnect right after the config commit can no longer cancel the delete and orphan now-unreferenced vault entries. - tests: assert GetConfig succeeds before treating an empty map as proof that nothing persisted (a swallowed read error would be a false pass). - web: on a post-save refetch failure, rebase initialValues onto the masked snapshot so the saved form no longer reads as dirty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): fail tests on discarded GetConfig errors, drop setState-in-updater - config_test: capture and fatal on GetConfig errors at the four remaining positive-assertion sites (a nil from a failed read would panic on the next map access instead of giving a clear failure), matching the earlier fix. - web hook: compute the masked post-save snapshot from the current values and apply it to setValues + setInitialValues directly, instead of calling setInitialValues inside a setValues functional updater (a React anti-pattern that runs twice under StrictMode). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugins): skip validation of a secret field's own vault ref - validateConfigSchema takes the plugin id and skips a secret field whose value is that field's config vault ref (an internal storage marker, already validated when first set). Without this, a field declaring both secret: true and enum: [...] 400s on every save that keeps the mask, because the enum check saw the vault ref instead of a user value. Freshly entered secret values are still validated; a foreign plugin's ref is not skipped. Covered by unit + service-level regression tests. - surface mid-loop vault rollback failures in storeConfigSecrets via a warning instead of discarding them (double-fault observability). - web: integer inputs get step=1 so browsers flag non-integral input in place (serializeConfigValues remains the backstop). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(plugins): split config secret tests into config_secrets_test.go config_test.go had grown past the 800-line backend file-length limit (revive), failing CI lint. Move the vault-backed config and plugin-scoped secret tests (plus their fake-vault/store helpers) into a sibling file; no test logic changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
233 lines
7.0 KiB
Go
233 lines
7.0 KiB
Go
package pluginsdk
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"net"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
"google.golang.org/grpc"
|
|
"google.golang.org/grpc/credentials/insecure"
|
|
"google.golang.org/grpc/test/bufconn"
|
|
)
|
|
|
|
// recordingHost is a fake Go-native Host implementation used to drive the
|
|
// grpcHostServer adapter without a real state store. It embeds
|
|
// UnimplementedHostData so it satisfies the extended Host interface without
|
|
// wiring the Host data API (ADR 0043) accessors — those are covered by
|
|
// dataRecordingHost in host_data_test.go.
|
|
type recordingHost struct {
|
|
UnimplementedHostData
|
|
getStateFn func(ctx context.Context, scope, scopeID, key string) (map[string]any, bool, error)
|
|
setState struct {
|
|
scope, scopeID, key string
|
|
value map[string]any
|
|
}
|
|
listStateEntries []StateEntry
|
|
configValues map[string]any
|
|
ownedSecrets map[string]string
|
|
revealSecretFn func(ctx context.Context, ref string) (string, error)
|
|
emitEvent struct {
|
|
name string
|
|
payload map[string]any
|
|
}
|
|
deleteStateCalled bool
|
|
}
|
|
|
|
func (h *recordingHost) GetState(ctx context.Context, scope, scopeID, key string) (map[string]any, bool, error) {
|
|
return h.getStateFn(ctx, scope, scopeID, key)
|
|
}
|
|
|
|
func (h *recordingHost) SetState(_ context.Context, scope, scopeID, key string, value map[string]any) error {
|
|
h.setState.scope, h.setState.scopeID, h.setState.key, h.setState.value = scope, scopeID, key, value
|
|
return nil
|
|
}
|
|
|
|
func (h *recordingHost) DeleteState(context.Context, string, string, string) error {
|
|
h.deleteStateCalled = true
|
|
return nil
|
|
}
|
|
|
|
func (h *recordingHost) ListState(context.Context, string, string) ([]StateEntry, error) {
|
|
return h.listStateEntries, nil
|
|
}
|
|
|
|
func (h *recordingHost) GetConfig(context.Context) (map[string]any, error) {
|
|
return h.configValues, nil
|
|
}
|
|
|
|
func (h *recordingHost) RevealSecret(ctx context.Context, ref string) (string, error) {
|
|
return h.revealSecretFn(ctx, ref)
|
|
}
|
|
|
|
func (h *recordingHost) GetSecret(_ context.Context, key string) (string, bool, error) {
|
|
value, ok := h.ownedSecrets[key]
|
|
return value, ok, nil
|
|
}
|
|
|
|
func (h *recordingHost) SetSecret(_ context.Context, key, value string) error {
|
|
if h.ownedSecrets == nil {
|
|
h.ownedSecrets = map[string]string{}
|
|
}
|
|
h.ownedSecrets[key] = value
|
|
return nil
|
|
}
|
|
|
|
func (h *recordingHost) DeleteSecret(_ context.Context, key string) error {
|
|
delete(h.ownedSecrets, key)
|
|
return nil
|
|
}
|
|
|
|
func (h *recordingHost) EmitEvent(_ context.Context, name string, payload map[string]any) error {
|
|
h.emitEvent.name, h.emitEvent.payload = name, payload
|
|
return nil
|
|
}
|
|
|
|
// dialHostOverBufconn wires a grpcHostServer (wrapping impl) to a
|
|
// grpcHostClient over an in-memory bufconn listener, bypassing go-plugin's
|
|
// broker entirely. The broker handshake itself is covered by the real
|
|
// end-to-end test in serve_test.go.
|
|
func dialHostOverBufconn(t *testing.T, impl Host) Host {
|
|
t.Helper()
|
|
|
|
lis := bufconn.Listen(1024 * 1024)
|
|
srv := grpc.NewServer()
|
|
registerHostServer(srv, impl)
|
|
go func() { _ = srv.Serve(lis) }()
|
|
t.Cleanup(srv.Stop)
|
|
|
|
conn, err := grpc.NewClient("passthrough:///bufconn",
|
|
grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) {
|
|
return lis.DialContext(ctx)
|
|
}),
|
|
grpc.WithTransportCredentials(insecure.NewCredentials()),
|
|
)
|
|
require.NoError(t, err)
|
|
t.Cleanup(func() { _ = conn.Close() })
|
|
|
|
return newHostClient(conn)
|
|
}
|
|
|
|
func TestHost_GetState_FoundAndNotFound(t *testing.T) {
|
|
impl := &recordingHost{
|
|
getStateFn: func(_ context.Context, scope, scopeID, key string) (map[string]any, bool, error) {
|
|
if scope == "task" && scopeID == "t1" && key == "k1" {
|
|
return map[string]any{"v": float64(1)}, true, nil
|
|
}
|
|
return nil, false, nil
|
|
},
|
|
}
|
|
host := dialHostOverBufconn(t, impl)
|
|
|
|
value, found, err := host.GetState(context.Background(), "task", "t1", "k1")
|
|
require.NoError(t, err)
|
|
require.True(t, found)
|
|
require.Equal(t, map[string]any{"v": float64(1)}, value)
|
|
|
|
_, found, err = host.GetState(context.Background(), "task", "t1", "missing")
|
|
require.NoError(t, err)
|
|
require.False(t, found)
|
|
}
|
|
|
|
func TestHost_SetState(t *testing.T) {
|
|
impl := &recordingHost{}
|
|
host := dialHostOverBufconn(t, impl)
|
|
|
|
err := host.SetState(context.Background(), "workspace", "ws1", "cfg", map[string]any{"enabled": true})
|
|
require.NoError(t, err)
|
|
require.Equal(t, "workspace", impl.setState.scope)
|
|
require.Equal(t, "ws1", impl.setState.scopeID)
|
|
require.Equal(t, "cfg", impl.setState.key)
|
|
require.Equal(t, map[string]any{"enabled": true}, impl.setState.value)
|
|
}
|
|
|
|
func TestHost_DeleteState(t *testing.T) {
|
|
impl := &recordingHost{}
|
|
host := dialHostOverBufconn(t, impl)
|
|
|
|
require.NoError(t, host.DeleteState(context.Background(), "task", "t1", "k1"))
|
|
require.True(t, impl.deleteStateCalled)
|
|
}
|
|
|
|
func TestHost_ListState(t *testing.T) {
|
|
impl := &recordingHost{listStateEntries: []StateEntry{
|
|
{Key: "a", Value: map[string]any{"x": float64(1)}, UpdatedAt: "2026-07-15T00:00:00Z"},
|
|
{Key: "b", Value: nil, UpdatedAt: "2026-07-15T00:00:01Z"},
|
|
}}
|
|
host := dialHostOverBufconn(t, impl)
|
|
|
|
entries, err := host.ListState(context.Background(), "instance", "")
|
|
require.NoError(t, err)
|
|
require.Equal(t, impl.listStateEntries, entries)
|
|
}
|
|
|
|
func TestHost_GetConfig(t *testing.T) {
|
|
impl := &recordingHost{configValues: map[string]any{
|
|
"github_token": "ghp_real",
|
|
"org": "kdlbs",
|
|
"max_items": float64(25),
|
|
}}
|
|
host := dialHostOverBufconn(t, impl)
|
|
|
|
config, err := host.GetConfig(context.Background())
|
|
require.NoError(t, err)
|
|
require.Equal(t, impl.configValues, config)
|
|
}
|
|
|
|
func TestHost_GetConfig_EmptyIsNonNil(t *testing.T) {
|
|
host := dialHostOverBufconn(t, &recordingHost{})
|
|
|
|
config, err := host.GetConfig(context.Background())
|
|
require.NoError(t, err)
|
|
require.NotNil(t, config)
|
|
require.Empty(t, config)
|
|
}
|
|
|
|
func TestHost_RevealSecret(t *testing.T) {
|
|
impl := &recordingHost{revealSecretFn: func(_ context.Context, ref string) (string, error) {
|
|
if ref == "known" {
|
|
return "sh-secret", nil
|
|
}
|
|
return "", errors.New("not found")
|
|
}}
|
|
host := dialHostOverBufconn(t, impl)
|
|
|
|
value, err := host.RevealSecret(context.Background(), "known")
|
|
require.NoError(t, err)
|
|
require.Equal(t, "sh-secret", value)
|
|
|
|
_, err = host.RevealSecret(context.Background(), "unknown")
|
|
require.Error(t, err)
|
|
}
|
|
|
|
func TestHost_PluginSecrets_RoundTripOverWire(t *testing.T) {
|
|
impl := &recordingHost{}
|
|
host := dialHostOverBufconn(t, impl)
|
|
ctx := context.Background()
|
|
|
|
_, found, err := host.GetSecret(ctx, "pat")
|
|
require.NoError(t, err)
|
|
require.False(t, found)
|
|
|
|
require.NoError(t, host.SetSecret(ctx, "pat", "ghp_owned"))
|
|
value, found, err := host.GetSecret(ctx, "pat")
|
|
require.NoError(t, err)
|
|
require.True(t, found)
|
|
require.Equal(t, "ghp_owned", value)
|
|
|
|
require.NoError(t, host.DeleteSecret(ctx, "pat"))
|
|
_, found, err = host.GetSecret(ctx, "pat")
|
|
require.NoError(t, err)
|
|
require.False(t, found)
|
|
}
|
|
|
|
func TestHost_EmitEvent(t *testing.T) {
|
|
impl := &recordingHost{}
|
|
host := dialHostOverBufconn(t, impl)
|
|
|
|
require.NoError(t, host.EmitEvent(context.Background(), "custom.thing", map[string]any{"a": float64(1)}))
|
|
require.Equal(t, "custom.thing", impl.emitEvent.name)
|
|
require.Equal(t, map[string]any{"a": float64(1)}, impl.emitEvent.payload)
|
|
}
|