From 574ef45b778330f918961ab787554ac74af2ed5e Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 17 Sep 2026 07:22:28 -0400 Subject: [PATCH 1/2] fix(plugin): let free users install the free half of a tier pair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nself plugin install cron` and `nself plugin install notify` failed for every operator without a license key: error installing "cron": plugin "cron" requires a license key Both slugs are served TWICE by the registry — once free, once pro — and ResolvePlugin already picks correctly between them by entitlement, defaulting to free. It never got the chance. installLocked's Step 1 ran `if isPaidPlugin(name) { checkLicense(...) }` BEFORE fetching the registry, off the static 59-name paidPlugins allowlist. Tier is not a property of a name, so that check cannot answer the question for a tier pair; it saw "cron" in the map and refused. The two plugins this locked out are exactly the two the published ɳTask docs advertise as free ("Powered by the free cron and notify plugins, no pro plugin required"). Fix: move the license check below tier resolution and drive it from the resolved manifest via isPaidPluginManifest, which reads the registry's own tier / requires_license fields. This also restores the load order manager.go documents as frozen (registry fetch -> license check -> checksum verify). Gating gets STRICTER, not looser: the static allowlist covered 21 of the 44 licensed-only plugins, so the other 23 — claw-budget, claw-news, family, mcp and friends — were never license-checked at Step 1 at all. Registry metadata covers all 44. Verified on a clean isolated HOME against the live registry: cron and notify install and land tier=free requires_license=false, while ai, activity-feed, claw-news, mcp and family are all still refused without a key. internal/bundle/installer_helpers.go carried the same drift, AND-ing the name map into a manifest check so a wholly free bundle containing cron or notify demanded a key. The manifest field alone is authoritative there. installer_locked.go was exactly at the 300-line cap, so the post-install reporting tail moves to installer_finish.go as a pure move rather than raising the budget. --- internal/bundle/installer_helpers.go | 9 +- internal/plugin/installer_finish.go | 39 ++++++++ .../plugin/installer_license_gate_test.go | 97 +++++++++++++++++++ internal/plugin/installer_locked.go | 43 ++++---- 4 files changed, 160 insertions(+), 28 deletions(-) create mode 100644 internal/plugin/installer_finish.go create mode 100644 internal/plugin/installer_license_gate_test.go diff --git a/internal/bundle/installer_helpers.go b/internal/bundle/installer_helpers.go index ddf25998..deb59c41 100644 --- a/internal/bundle/installer_helpers.go +++ b/internal/bundle/installer_helpers.go @@ -150,8 +150,13 @@ func defaultLicenseChecker(ctx context.Context, plugins []string) error { // don't pre-error here so we get the proper "not found" message. continue } - // Free plugins don't need a license. - if !m.RequiresLicense && !plugin.IsPaidPlugin(name) { + // Free plugins don't need a license. The resolved manifest's own + // requires_license field is authoritative; the paidPlugins name map + // that used to be AND-ed in here is a pre-registry fallback that has + // drifted, and for a tier_pair slug (cron, notify) it reports "paid" + // even when the entry actually being installed is the free one — + // which made a wholly free bundle demand a license key. + if !m.RequiresLicense { continue } // Paid plugin: ensure at least one license key is set. We don't diff --git a/internal/plugin/installer_finish.go b/internal/plugin/installer_finish.go new file mode 100644 index 00000000..8e9a7223 --- /dev/null +++ b/internal/plugin/installer_finish.go @@ -0,0 +1,39 @@ +package plugin + +// installer_finish.go — the post-install reporting tail of installLocked. +// +// Purpose: emit the permission audit log, warn on dangerous permissions, +// print the "run nself build" hint, and fire the anonymous install event. +// Inputs: the plugin name and its resolved registry manifest. +// Outputs: none — every step here is reporting, and none can fail the +// install, which has already succeeded by the time this runs. +// Constraints: split out of installer_locked.go to keep that file within the +// 300-line cap; a pure move, same order, no behaviour change. + +import ( + "fmt" + "log/slog" + "os" +) + +// finishInstall runs the reporting tail of a successful plugin install. +func finishInstall(name string, manifest *PluginManifest) { + // S71-T02: Emit structured audit log for the granted permission set. + // One line per install, consumable by Loki. Never logs secret values — + // only the permission strings declared in the manifest. + slog.Info("plugin.install.permissions", + "plugin", name, + "version", manifest.Version, + "permissions", manifest.Permissions.Strings(), + ) + + // S71-T02: Warn via doctor when dangerous permissions are present. + logDangerousPermissions(name, manifest.Permissions.Strings()) + + fmt.Fprintf(os.Stderr, "\n\u2139 Run 'nself build' to include %s in your stack.\n", name) + + // S68-T02: Fire-and-forget install-event to plugins.nself.org registry. + // Silent, 1s timeout, never blocks the install. Sends only an opaque + // SHA-256 hash of the machine fingerprint — no PII in the payload. + go postInstallEvent(name) +} diff --git a/internal/plugin/installer_license_gate_test.go b/internal/plugin/installer_license_gate_test.go new file mode 100644 index 00000000..e3ca9157 --- /dev/null +++ b/internal/plugin/installer_license_gate_test.go @@ -0,0 +1,97 @@ +package plugin + +// installer_license_gate_test.go — regression coverage for WHERE the install +// license gate runs. +// +// The defect: installLocked gated on isPaidPlugin(name), a static 59-name +// allowlist, BEFORE fetching the registry and resolving the tier. Tier is not +// a property of a name — "cron" and "notify" are each served twice, free and +// pro — so an unlicensed operator was refused before ResolvePlugin could pick +// the free entry, making the two plugins the ɳTask docs advertise as free +// impossible to install. tier_resolve_test.go covers resolution itself; these +// tests cover the gate that resolution feeds. + +import ( + "context" + "os" + "strings" + "testing" +) + +// TestInstallGate_TierPairFreeEntryIsNotPaid pins the exact composition that +// was broken: an unentitled operator resolves a tier pair to the FREE entry, +// and that entry must not then be treated as license-gated. +func TestInstallGate_TierPairFreeEntryIsNotPaid(t *testing.T) { + m, err := ResolvePlugin(context.Background(), fixtureRegistry(), "cron", "", neverEntitled) + if err != nil { + t.Fatalf("resolving cron without entitlement: %v", err) + } + if m.Tier != "free" { + t.Fatalf("expected the free entry, got tier=%q", m.Tier) + } + if isPaidPluginManifest(m) { + t.Error("the free half of a tier pair was classified as paid; an unlicensed operator cannot install it") + } + // The name-based map disagrees — that disagreement is the whole bug, so + // assert it explicitly rather than leaving it implicit. + if !isPaidPlugin("cron") { + t.Skip("paidPlugins no longer lists cron; this test's premise has changed") + } +} + +// TestInstallGate_EntitledTierPairProEntryIsPaid is the other half: when +// entitlement does resolve to pro, the gate must still fire. +func TestInstallGate_EntitledTierPairProEntryIsPaid(t *testing.T) { + m, err := ResolvePlugin(context.Background(), fixtureRegistry(), "cron", "", alwaysEntitled) + if err != nil { + t.Fatalf("resolving cron with entitlement: %v", err) + } + if !isPaidPluginManifest(m) { + t.Errorf("the pro half of a tier pair must stay license-gated, got tier=%q", m.Tier) + } +} + +// TestInstallGate_ProOnlyPluginIsPaidRegardlessOfAllowlist covers the +// tightening the move buys: a pro registry entry is gated on its own +// metadata, including the 23 paid plugins the static allowlist omits. +func TestInstallGate_ProOnlyPluginIsPaidRegardlessOfAllowlist(t *testing.T) { + m, err := ResolvePlugin(context.Background(), fixtureRegistry(), "search-pro-only", "", neverEntitled) + if err != nil { + t.Fatalf("resolving search-pro-only: %v", err) + } + if !isPaidPluginManifest(m) { + t.Error("a pro-tier entry must be license-gated even though it is absent from paidPlugins") + } + if isPaidPlugin("search-pro-only") { + t.Fatal("fixture premise broken: this name should NOT be in the static allowlist") + } +} + +// TestInstallGate_NotGatedBeforeRegistryFetch is a structural guard. The gate +// must not move back above the registry fetch: at that point the tier of a +// tier_pair slug is simply unknown, so any name-based decision there is a +// guess. Reading the source is the only way to assert ordering without a +// live registry. +func TestInstallGate_NotGatedBeforeRegistryFetch(t *testing.T) { + src, err := os.ReadFile("installer_locked.go") + if err != nil { + t.Fatalf("reading installer_locked.go: %v", err) + } + body := string(src) + + fetch := strings.Index(body, "FetchRegistry(") + if fetch < 0 { + t.Fatal("FetchRegistry call not found; this guard needs updating") + } + gate := strings.Index(body, "isPaidPluginManifest(manifest)") + if gate < 0 { + t.Fatal("manifest-based license gate not found in installLocked") + } + if gate < fetch { + t.Error("the license gate runs before the registry fetch; tier is unknown there") + } + if i := strings.Index(body, "isPaidPlugin(name)"); i >= 0 { + t.Errorf("installLocked gates on the static name allowlist at offset %d; "+ + "use isPaidPluginManifest on the resolved manifest instead", i) + } +} diff --git a/internal/plugin/installer_locked.go b/internal/plugin/installer_locked.go index 62d12ccb..9c76c4a6 100644 --- a/internal/plugin/installer_locked.go +++ b/internal/plugin/installer_locked.go @@ -25,14 +25,12 @@ import ( // acquired the install lock. Dependency installs call this directly to avoid // attempting to re-acquire the lock (which would deadlock). func installLocked(ctx context.Context, cfg *config.Config, name string, pluginDir string) error { - // Step 1: License check for paid plugins. - if isPaidPlugin(name) { - if err := checkLicense(ctx, name); err != nil { - return err - } - } - - // Step 2: Fetch registry and locate the plugin. + // Step 1: Fetch registry and locate the plugin. The license check runs + // AFTER resolution (Step 2b), not here: tier is not a property of the + // NAME, so the static paidPlugins map cannot answer it for a tier_pair + // slug. cron and notify are each served twice (free and pro) and gating + // on the name rejected unlicensed operators before ResolvePlugin could + // pick the free entry. This is also the order manager.go calls frozen. cacheDir := defaultCacheDir() reg, err := FetchRegistry(ctx, "", cacheDir) if err != nil { @@ -51,6 +49,15 @@ func installLocked(ctx context.Context, cfg *config.Config, name string, pluginD return err } + // Step 2b: License check against the RESOLVED manifest. Reads the + // registry's own tier/requires_license fields, so it gates exactly what + // the registry publishes as paid — tighter than the drifted name map. + if isPaidPluginManifest(manifest) { + if err := checkLicense(ctx, name); err != nil { + return err + } + } + // Status check: lifecycle policy enforcement (S58-T01, S58-T02, S58-T03). // "stable" and "" (legacy, no status field) proceed silently — compared // via EffectiveStatus so both are handled by the same code path (FIX-CLI-6). @@ -277,24 +284,8 @@ func installLocked(ctx context.Context, cfg *config.Config, name string, pluginD // registration (split out — see installer_identity.go). registerPluginIdentityIfEnabled(ctx, pluginDir, name) - // S71-T02: Emit structured audit log for the granted permission set. - // One line per install, consumable by Loki. Never logs secret values — - // only the permission strings declared in the manifest. - slog.Info("plugin.install.permissions", - "plugin", name, - "version", manifest.Version, - "permissions", manifest.Permissions.Strings(), - ) - - // S71-T02: Warn via doctor when dangerous permissions are present. - logDangerousPermissions(name, manifest.Permissions.Strings()) - - fmt.Fprintf(os.Stderr, "\nℹ Run 'nself build' to include %s in your stack.\n", name) - - // S68-T02: Fire-and-forget install-event to plugins.nself.org registry. - // Silent, 1s timeout, never blocks the install. Sends only an opaque - // SHA-256 hash of the machine fingerprint — no PII in the payload. - go postInstallEvent(name) + // Step 8: post-install reporting (split out — see installer_finish.go). + finishInstall(name, manifest) return nil } From be695f65a89de2dd820e8b4389c8aca3922bbfd1 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 17 Sep 2026 08:06:02 -0400 Subject: [PATCH 2/2] test: pin the registry in the paid-plugin license test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestInstall_PaidPluginRequiresLicense reached the live registry over the network and only appeared to pass: the license gate ran off the paidPlugins NAME map before any fetch, so the registry response never mattered. With the gate moved below tier resolution it started failing on windows-2022, where the primary registry was unreachable and the GitHub raw fallback — the FREE registry — carries no "ai" entry, so the error became "plugin not found in registry" rather than a license error. Serve the registry from httptest instead, and set USERPROFILE alongside HOME. os.UserHomeDir reads USERPROFILE on Windows, so the old test left the real cache and license dir reachable there; that difference is why this passed locally on macOS and failed only in Windows CI. The test now exercises the path it claims to: fetch -> resolve -> gate. --- internal/plugin/install_test.go | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/internal/plugin/install_test.go b/internal/plugin/install_test.go index 5c6842ea..48f6135a 100644 --- a/internal/plugin/install_test.go +++ b/internal/plugin/install_test.go @@ -2,6 +2,7 @@ package plugin import ( "context" + "io" "net/http" "net/http/httptest" "os" @@ -212,19 +213,41 @@ func TestFetchRegistry_MockServer(t *testing.T) { // --- Install license check test --- // TestInstall_PaidPluginRequiresLicense verifies that Install() returns a -// license error when a paid plugin is requested without a valid license key set. -// This exercises the IsPaidPlugin + checkLicense path without requiring Docker. +// license error when a paid plugin is requested without a valid license key. +// +// The registry is served from a local httptest server rather than the live +// one. This test used to reach the network, and only appeared to pass: the +// license gate ran off the paidPlugins NAME map before any fetch, so the +// registry response never mattered. Once the gate moved below tier +// resolution the test started failing on the windows-2022 runner, where the +// primary registry was unreachable and the GitHub raw fallback — the FREE +// registry — has no "ai" entry, so the error became "plugin not found in +// registry". Pinning the registry makes the test hermetic and makes it +// exercise the path it claims to: fetch -> resolve -> license gate. func TestInstall_PaidPluginRequiresLicense(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"plugins":[ + {"name":"ai","version":"1.1.0","tier":"pro","requires_license":true} + ]}`) + })) + defer srv.Close() + // Ensure no license key is set. t.Setenv("NSELF_PLUGIN_LICENSE_KEY", "") t.Setenv("NSELF_LICENSE_SKIP_VERIFY", "") - // Set HOME to temp dir so no ~/.nself/license/key file is found. - t.Setenv("HOME", t.TempDir()) + t.Setenv("NSELF_PLUGIN_REGISTRY", srv.URL) + // Point the home dir at a temp dir so neither ~/.nself/license/key nor a + // populated registry cache leaks in. USERPROFILE is the Windows spelling + // os.UserHomeDir reads, and omitting it is why this passed locally on + // macOS while failing in Windows CI. + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginDir := t.TempDir() cfg := &config.Config{} - // "ai" is a paid plugin per paidPlugins map. err := Install(context.Background(), cfg, "ai", pluginDir) if err == nil { t.Fatal("expected error when installing paid plugin without license, got nil")