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/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") 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 }