From 23a3e7eea34ad5b97883233b68b8e323b8ef17ac Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:04:14 -0400 Subject: [PATCH 01/18] fix(entitlement): serialize timer and token writes (D-0947) --- cmd/commands/dispatch.go | 11 +-- internal/docker/file_bind_mount.go | 48 +++++++++++ internal/docker/file_bind_mount_test.go | 35 ++++++++ .../entitlement/atomic_refresh_test.go | 83 +++++++++++++++++++ internal/license/entitlement/installed.go | 2 +- internal/license/entitlement/mounted.go | 32 +++++++ internal/license/entitlement/write.go | 17 +++- .../entitlement_cron_lock_posix.go | 52 ++++++++++++ internal/maintenance/entitlement_timer.go | 11 +-- .../maintenance/entitlement_timer_posix.go | 20 ++--- .../maintenance/entitlement_timer_test.go | 60 +++++++++++++- .../testdata/entitlement_timer/crontab | 2 +- .../entitlement_timer/launchagent.plist | 2 +- .../entitlement_timer/systemd.service | 2 +- 14 files changed, 348 insertions(+), 29 deletions(-) create mode 100644 internal/docker/file_bind_mount.go create mode 100644 internal/docker/file_bind_mount_test.go create mode 100644 internal/license/entitlement/atomic_refresh_test.go create mode 100644 internal/license/entitlement/mounted.go create mode 100644 internal/maintenance/entitlement_cron_lock_posix.go diff --git a/cmd/commands/dispatch.go b/cmd/commands/dispatch.go index 346beea43..f4154255c 100644 --- a/cmd/commands/dispatch.go +++ b/cmd/commands/dispatch.go @@ -17,18 +17,18 @@ package commands import ( "errors" - "io" - "strings" - "fmt" - "github.com/spf13/pflag" + "io" "os" + "strings" "github.com/nself-org/cli/internal/compat" + "github.com/nself-org/cli/internal/docker" "github.com/nself-org/cli/internal/errs" + "github.com/nself-org/cli/internal/license/entitlement" "github.com/nself-org/cli/internal/plugin" - "github.com/spf13/cobra" + "github.com/spf13/pflag" ) // invokedCommandPath returns the command path as the user actually spelled it, @@ -62,6 +62,7 @@ func Execute() (resultErr error) { commandLogFinish = nil } }() + entitlement.SetMountProbe(docker.FileBindMounted) // Secret values arrive by environment, not argv (D-0468): restore them first. if err := restoreSecretArgv(); err != nil { return err diff --git a/internal/docker/file_bind_mount.go b/internal/docker/file_bind_mount.go new file mode 100644 index 000000000..8482ce22c --- /dev/null +++ b/internal/docker/file_bind_mount.go @@ -0,0 +1,48 @@ +package docker + +import ( + "context" + "encoding/json" + "fmt" + "os/exec" + "path/filepath" + "strings" +) + +// FileBindMounted reports whether any container binds this exact host file. +// Stopped containers count too. Unavailable or incomplete inspection is an error, +// so callers can preserve the inode whenever its mount status is uncertain. +func FileBindMounted(ctx context.Context, path string) (bool, error) { + ids, err := exec.CommandContext(ctx, "docker", "ps", "-aq").Output() + if err != nil { + return false, err + } + names := strings.Fields(string(ids)) + if len(names) == 0 { + return false, nil + } + data, err := exec.CommandContext(ctx, "docker", append([]string{"inspect"}, names...)...).Output() + if err != nil { + return false, err + } + var containers []struct{ Mounts []struct{ Source string } } + if err := json.Unmarshal(data, &containers); err != nil { + return false, err + } + target, err := filepath.EvalSymlinks(path) + if err != nil { + return false, fmt.Errorf("cannot resolve bind mount path: %w", err) + } + for _, container := range containers { + for _, mount := range container.Mounts { + source, err := filepath.EvalSymlinks(mount.Source) + if err != nil { + return false, fmt.Errorf("cannot resolve bind mount path: %w", err) + } + if source == target { + return true, nil + } + } + } + return false, nil +} diff --git a/internal/docker/file_bind_mount_test.go b/internal/docker/file_bind_mount_test.go new file mode 100644 index 000000000..55527140d --- /dev/null +++ b/internal/docker/file_bind_mount_test.go @@ -0,0 +1,35 @@ +//go:build !windows + +package docker + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" +) + +func TestFileBindMounted(t *testing.T) { + path := filepath.Join(t.TempDir(), "token") + if err := os.WriteFile(path, []byte("token"), 0600); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name, script string + mounted, fail bool + }{ + {"mounted", fmt.Sprintf("if [ \"$1\" = ps ]; then echo cid; else printf '%%s' '[{\"Mounts\":[{\"Source\":\"%s\"}]}]'; fi\n", path), true, false}, + {"no containers", "exit 0\n", false, false}, + {"unavailable", "exit 1\n", false, true}, + {"bad inspection", "echo cid\n", false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + fakeDockerOnPath(t, tc.script) + got, err := FileBindMounted(context.Background(), path) + if got != tc.mounted || (err != nil) != tc.fail { + t.Fatalf("got %v, %v", got, err) + } + }) + } +} diff --git a/internal/license/entitlement/atomic_refresh_test.go b/internal/license/entitlement/atomic_refresh_test.go new file mode 100644 index 000000000..349af7499 --- /dev/null +++ b/internal/license/entitlement/atomic_refresh_test.go @@ -0,0 +1,83 @@ +package entitlement + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + "time" +) + +func TestUnmountedRefreshReplacesInode(t *testing.T) { + previous := entitlementMounted + entitlementMounted = func(string) bool { return false } + defer func() { entitlementMounted = previous }() + path := filepath.Join(t.TempDir(), "entitlement.jwt") + if err := writeAtomic(path, []byte("first")); err != nil { + t.Fatal(err) + } + before, _ := os.Stat(path) + if err := writeAtomic(path, []byte("second")); err != nil { + t.Fatal(err) + } + after, _ := os.Stat(path) + if os.SameFile(before, after) { + t.Fatal("unmounted refresh rewrote the live inode") + } +} + +func TestConcurrentEntitlementWritesSerialize(t *testing.T) { + path := filepath.Join(t.TempDir(), "entitlement.jwt") + if err := writeAtomic(path, []byte("first")); err != nil { + t.Fatal(err) + } + lock, err := os.OpenFile(filepath.Join(filepath.Dir(path), ".entitlement-write.lock"), os.O_CREATE|os.O_RDWR, 0600) + if err != nil { + t.Fatal(err) + } + defer func() { _ = lock.Close() }() + if err := lockRejected(lock); err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { done <- writeAtomic(path, []byte("second")) }() + select { + case err := <-done: + t.Fatalf("writer bypassed held publication lock: %v", err) + case <-time.After(50 * time.Millisecond): + } + _ = lock.Close() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("writer did not resume after publication lock released") + } +} + +func TestMountProbePolicy(t *testing.T) { + mountProbeMu.RLock() + previous := mountProbe + mountProbeMu.RUnlock() + defer SetMountProbe(previous) + for _, tc := range []struct { + name string + probe func(context.Context, string) (bool, error) + want bool + }{ + {"unconfigured", nil, true}, + {"unmounted", func(context.Context, string) (bool, error) { return false, nil }, false}, + {"mounted", func(context.Context, string) (bool, error) { return true, nil }, true}, + {"unavailable", func(context.Context, string) (bool, error) { return false, errors.New("offline") }, true}, + } { + t.Run(tc.name, func(t *testing.T) { + SetMountProbe(tc.probe) + if got := entitlementMounted("token"); got != tc.want { + t.Fatalf("mounted = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/internal/license/entitlement/installed.go b/internal/license/entitlement/installed.go index 8c79c2b74..2f63e366a 100644 --- a/internal/license/entitlement/installed.go +++ b/internal/license/entitlement/installed.go @@ -19,7 +19,7 @@ var ErrValidationUnavailable = errors.New("could not validate the new licence on // RefreshInstalled refreshes a project only when a licensed plugin is installed. // Purpose: keep free-only stacks independent of licences. Inputs: context/project. -// Outputs: the refresh result. Constraints: native manifests, no runtime Docker calls. +// Outputs: the refresh result. Constraints: native manifests; optional host probe preserves mounted inodes. // SPORT: start/license-set entitlement publication. func RefreshInstalled(ctx context.Context, projectDir string) error { return RefreshInstalledForKey(ctx, projectDir, "") diff --git a/internal/license/entitlement/mounted.go b/internal/license/entitlement/mounted.go new file mode 100644 index 000000000..31adb63b6 --- /dev/null +++ b/internal/license/entitlement/mounted.go @@ -0,0 +1,32 @@ +package entitlement + +import ( + "context" + "sync" + "time" +) + +var mountProbeMu sync.RWMutex +var mountProbe func(context.Context, string) (bool, error) + +// SetMountProbe supplies host inspection from the application's composition +// root. Library users without a probe conservatively preserve existing inodes. +func SetMountProbe(probe func(context.Context, string) (bool, error)) { + mountProbeMu.Lock() + defer mountProbeMu.Unlock() + mountProbe = probe +} + +// Preserve mounted inodes, including when the host cannot establish their status. +var entitlementMounted = func(path string) bool { + mountProbeMu.RLock() + probe := mountProbe + mountProbeMu.RUnlock() + if probe == nil { + return true + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + mounted, err := probe(ctx, path) + return err != nil || mounted +} diff --git a/internal/license/entitlement/write.go b/internal/license/entitlement/write.go index ab5de5f45..c8e2ead9e 100644 --- a/internal/license/entitlement/write.go +++ b/internal/license/entitlement/write.go @@ -120,7 +120,8 @@ func cachedToken() (string, error) { return "", fmt.Errorf("cached token does not belong to a configured licence") } -// writeAtomic creates new files atomically and refreshes existing mounted inodes. +// writeAtomic serializes writers and atomically replaces files known to be +// unmounted. Mounted files and uncertain host state retain the existing inode. // Purpose: keep single-file bind mounts current. Inputs: path/data. Outputs: error. // Constraints: 0600; SDK retries torn reads. SPORT: private state publication. func writeAtomic(path string, data []byte) error { @@ -131,6 +132,14 @@ func writeAtomic(path string, data []byte) error { if err := os.MkdirAll(dir, 0700); err != nil { return err } + lock, err := os.OpenFile(filepath.Join(dir, ".entitlement-write.lock"), os.O_CREATE|os.O_RDWR|noFollowFlag, 0600) + if err != nil { + return err + } + defer func() { _ = lock.Close() }() + if err := lockRejected(lock); err != nil { + return err + } info, err := os.Lstat(path) if err == nil { if info.IsDir() { @@ -138,6 +147,12 @@ func writeAtomic(path string, data []byte) error { return fmt.Errorf("entitlement target directory is not empty or cannot be removed: %w", err) } } else { + if !info.Mode().IsRegular() || !singleLink(info) { + return writeExisting(path, data, info) + } + if !entitlementMounted(path) { + return writeNew(path, data) + } return writeExisting(path, data, info) } } else if !os.IsNotExist(err) { diff --git a/internal/maintenance/entitlement_cron_lock_posix.go b/internal/maintenance/entitlement_cron_lock_posix.go new file mode 100644 index 000000000..8de27968a --- /dev/null +++ b/internal/maintenance/entitlement_cron_lock_posix.go @@ -0,0 +1,52 @@ +//go:build darwin || linux + +package maintenance + +import ( + "os" + "path/filepath" + "strings" + "syscall" +) + +func (h host) removeCron(hash string) (bool, error) { + if h.goos != "linux" || !h.crontab { + return false, nil + } + unlock, err := h.lockCron() + if err != nil { + return false, err + } + defer unlock() + cur, err := h.cronList() + if err != nil { + return false, err + } + rest, tagged := taggedLines(cur, entCronTag+hash) + if len(tagged) == 0 { + return false, nil + } + out, err := h.run(strings.Join(rest, "\n")+"\n", "crontab", "-") + if err != nil { + return false, cmdErr(out, err, "crontab", "-") + } + return true, nil +} + +// lockCron shares the lock with the cron self-removal shell. Never unlink it: +// waiters must continue to lock the same inode. +func (h host) lockCron() (func(), error) { + dir := filepath.Join(h.home, ".nself") + if err := os.MkdirAll(dir, 0700); err != nil { + return nil, err + } + f, err := os.OpenFile(filepath.Join(dir, "entitlement-cron.lock"), os.O_CREATE|os.O_RDWR|syscall.O_NOFOLLOW, 0600) + if err != nil { + return nil, err + } + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { + _ = f.Close() + return nil, err + } + return func() { _ = f.Close() }, nil +} diff --git a/internal/maintenance/entitlement_timer.go b/internal/maintenance/entitlement_timer.go index 0f69ece2c..9775d87ef 100644 --- a/internal/maintenance/entitlement_timer.go +++ b/internal/maintenance/entitlement_timer.go @@ -96,9 +96,10 @@ func systemdEscape(s string) string { // cronEscape escapes `%`, which cron turns into a newline. func cronEscape(s string) string { return strings.ReplaceAll(s, "%", `\%`) } -// refreshScript is the guard + refresh command shared by systemd and cron text. +// refreshScript guards presence, not executability: an upgrade can temporarily +// clear executable bits. Only a missing binary or project retires the timer. func refreshScript(bin, dir string, gone string) string { - return fmt.Sprintf("test -x %s && test -d %s || %s", shQuote(bin), shQuote(dir), gone) + return fmt.Sprintf("test -e %s && test -d %s || %s", shQuote(bin), shQuote(dir), gone) } // refreshCmd pins v1.5 mode: the timer is only installed in it, and its environment is empty. @@ -162,8 +163,8 @@ func renderCron(bin, dir, hash string) (string, error) { } minute, _ := strconv.ParseUint(hash[:2], 16, 8) tag := entCronTag + hash - drop := fmt.Sprintf(`t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -`, shQuote(tag)) - line := fmt.Sprintf("%d */6 * * * if test -x %s && test -d %s; then %s >/dev/null; else %s; fi %s", + drop := fmt.Sprintf(`(flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"`, shQuote(tag)) + line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) return cronEscape(line), nil } @@ -177,7 +178,7 @@ func renderPlist(bin, dir, hash string) ([]byte, error) { return nil, err } label := launchAgentLabel(hash) - script := fmt.Sprintf(`if test -x %s && test -d %s; then exec %s; else rm -f "$HOME/Library/LaunchAgents/%s.plist"; launchctl bootout "gui/$(id -u)/%s"; fi`, + script := fmt.Sprintf(`if test -e %s && test -d %s; then exec %s; else rm -f "$HOME/Library/LaunchAgents/%s.plist"; launchctl bootout "gui/$(id -u)/%s"; fi`, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), label, label) var sb strings.Builder sb.WriteString(xml.Header) diff --git a/internal/maintenance/entitlement_timer_posix.go b/internal/maintenance/entitlement_timer_posix.go index dc0df28c5..f24a50db2 100644 --- a/internal/maintenance/entitlement_timer_posix.go +++ b/internal/maintenance/entitlement_timer_posix.go @@ -186,6 +186,11 @@ func (h host) installSystemd(bin, dir, hash string, uid uint32) (bool, error) { } func (h host) installCron(bin, dir, hash string) (bool, error) { + unlock, err := h.lockCron() + if err != nil { + return false, err + } + defer unlock() line, err := renderCron(bin, dir, hash) if err != nil { return false, err @@ -278,18 +283,9 @@ func (h host) remove(hash string) (r RemoveResult, firstErr error) { r.Removed = true } } - cur, cerr := h.cronList() - if cerr != nil && cerr != ErrNoBackend { // no crontab command or not Linux: nothing to read - fail(cerr) // an unreadable crontab may still hold our line - } - if rest, tagged := taggedLines(cur, entCronTag+hash); cerr == nil && len(tagged) > 0 { - out, err := h.run(strings.Join(rest, "\n")+"\n", "crontab", "-") - if err != nil { - fail(cmdErr(out, err, "crontab", "-")) - } else { - r.Removed = true - } - } + removed, err := h.removeCron(hash) + fail(err) + r.Removed = r.Removed || removed return r, firstErr } diff --git a/internal/maintenance/entitlement_timer_test.go b/internal/maintenance/entitlement_timer_test.go index def09ae71..118093ca6 100644 --- a/internal/maintenance/entitlement_timer_test.go +++ b/internal/maintenance/entitlement_timer_test.go @@ -10,8 +10,11 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" + "sync" "testing" + "time" "unicode/utf8" ) @@ -368,7 +371,7 @@ func TestLaunchAgentGuard(t *testing.T) { } args := plistArgs(t, data) cmd := exec.Command(args[0], args[1:]...) - cmd.Env = []string{"HOME=" + home, "PATH=" + stubs + ":/usr/bin:/bin"} + cmd.Env = []string{"HOME=" + home, "PATH=" + stubs + ":" + os.Getenv("PATH")} if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("guard script failed: %v\n%s", err, out) } @@ -524,7 +527,16 @@ func TestCronSelfDeleteKeepsForeignLines(t *testing.T) { } fields := strings.SplitN(strings.ReplaceAll(line, `\%`, "%"), " ", 6) run := exec.Command("sh", "-c", fields[5]) - run.Env = []string{"PATH=" + stubs + ":/usr/bin:/bin"} + if err := os.MkdirAll(filepath.Join(tmp, ".nself"), 0700); err != nil { + t.Fatal(err) + } + // This shell shape is also tested on macOS, where flock is not shipped. + if runtime.GOOS == "darwin" { + if err := os.WriteFile(filepath.Join(stubs, "flock"), []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil { + t.Fatal(err) + } + } + run.Env = []string{"HOME=" + tmp, "PATH=" + stubs + ":" + os.Getenv("PATH")} if out, err := run.CombinedOutput(); err != nil { t.Fatalf("cron line failed: %v\n%s", err, out) } @@ -583,3 +595,47 @@ func TestOverlongPathsAreRefused(t *testing.T) { t.Fatal("a cron line near cron's length limit was rendered") } } + +func TestCronConcurrentProjectsKeepBothLines(t *testing.T) { + f, root := newFake(t, "linux", 1000) + var mu sync.Mutex + current := "" + h := f.host + h.run = func(stdin, name string, args ...string) (string, error) { + mu.Lock() + if args[0] == "-l" { + value := current + mu.Unlock() + time.Sleep(20 * time.Millisecond) + return value, nil + } + current = stdin + mu.Unlock() + return "", nil + } + var wg sync.WaitGroup + for _, name := range []string{"one", "two"} { + wg.Add(1) + go func(name string) { + defer wg.Done() + if _, err := h.installCron("/bin/nself", filepath.Join(root, name), Hash(name)); err != nil { + t.Error(err) + } + }(name) + } + wg.Wait() + if strings.Count(current, entCronTag) != 4 { + t.Fatalf("lost a project cron line: %s", current) + } +} + +func TestRefreshGuardRetainsNonExecutableBinary(t *testing.T) { + root := t.TempDir() + bin := filepath.Join(root, "nself") + if err := os.WriteFile(bin, []byte("upgrading"), 0600); err != nil { + t.Fatal(err) + } + if out, err := exec.Command("sh", "-c", refreshScript(bin, root, "exit 78")).CombinedOutput(); err != nil { + t.Fatalf("existing binary treated as gone: %v %s", err, out) + } +} diff --git a/internal/maintenance/testdata/entitlement_timer/crontab b/internal/maintenance/testdata/entitlement_timer/crontab index 40c2270df..042914c97 100644 --- a/internal/maintenance/testdata/entitlement_timer/crontab +++ b/internal/maintenance/testdata/entitlement_timer/crontab @@ -1 +1 @@ -38 */6 * * * if test -x '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | crontab -; fi # nself-entitlement:dae668e4084f07b6 +38 */6 * * * if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else (flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"; fi # nself-entitlement:dae668e4084f07b6 diff --git a/internal/maintenance/testdata/entitlement_timer/launchagent.plist b/internal/maintenance/testdata/entitlement_timer/launchagent.plist index 09a83a254..d995e0ec4 100644 --- a/internal/maintenance/testdata/entitlement_timer/launchagent.plist +++ b/internal/maintenance/testdata/entitlement_timer/launchagent.plist @@ -8,7 +8,7 @@ /bin/sh -c - if test -x '/usr/local/bin/nself' && test -d '/srv/app'; then exec env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet; else rm -f "$HOME/Library/LaunchAgents/org.nself.entitlement.dae668e4084f07b6.plist"; launchctl bootout "gui/$(id -u)/org.nself.entitlement.dae668e4084f07b6"; fi + if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then exec env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet; else rm -f "$HOME/Library/LaunchAgents/org.nself.entitlement.dae668e4084f07b6.plist"; launchctl bootout "gui/$(id -u)/org.nself.entitlement.dae668e4084f07b6"; fi StartInterval 21600 diff --git a/internal/maintenance/testdata/entitlement_timer/systemd.service b/internal/maintenance/testdata/entitlement_timer/systemd.service index 8301b9362..34c40a39c 100644 --- a/internal/maintenance/testdata/entitlement_timer/systemd.service +++ b/internal/maintenance/testdata/entitlement_timer/systemd.service @@ -5,5 +5,5 @@ Description=nSelf entitlement refresh dae668e4084f07b6 Type=oneshot User=1000 SuccessExitStatus=78 -ExecStart=/bin/sh -c 'test -x \'/usr/local/bin/nself\' && test -d \'/srv/app\' || exit 78; exec env NSELF_V15=1 \'/usr/local/bin/nself\' license refresh --project \'/srv/app\' --quiet' +ExecStart=/bin/sh -c 'test -e \'/usr/local/bin/nself\' && test -d \'/srv/app\' || exit 78; exec env NSELF_V15=1 \'/usr/local/bin/nself\' license refresh --project \'/srv/app\' --quiet' ExecStopPost=+/bin/sh -c 'if test "$$EXIT_STATUS" = 78; then systemctl disable --now nself-entitlement-dae668e4084f07b6.timer; rm -f /etc/systemd/system/nself-entitlement-dae668e4084f07b6.service /etc/systemd/system/nself-entitlement-dae668e4084f07b6.timer; systemctl daemon-reload; fi' From bae90590e8f44d878da8c82a3aec88c77c385e76 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:04:30 -0400 Subject: [PATCH 02/18] test(plugin): pin archive snapshot contract from D-1326 (D-0844) This commit adds only a regression test for openArchive, verifyChecksumSum and extractTarGzFrom, which were already fixed on origin/main in commit 4628857c (D-1326). The debt row resolution should point at that earlier commit; this test pins the contract so future refactors cannot regress it. --- internal/plugin/archive_snapshot_test.go | 53 ++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 internal/plugin/archive_snapshot_test.go diff --git a/internal/plugin/archive_snapshot_test.go b/internal/plugin/archive_snapshot_test.go new file mode 100644 index 000000000..b2d59b3f2 --- /dev/null +++ b/internal/plugin/archive_snapshot_test.go @@ -0,0 +1,53 @@ +package plugin + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "os" + "path/filepath" + "testing" +) + +// TestArchiveSnapshotSurvivesPathReplacement pins the D-0844 / D-1326 contract: +// openArchive hashes the downloaded file once, unlinks the path, and returns +// a handle rewound to the start. Checksum verification and extraction both +// read that handle, so replacing the path after the open cannot change what +// gets verified or extracted. +func TestArchiveSnapshotSurvivesPathReplacement(t *testing.T) { + var archive bytes.Buffer + gz := gzip.NewWriter(&archive) + tw := tar.NewWriter(gz) + _ = tw.WriteHeader(&tar.Header{Name: "payload", Mode: 0600, Size: 7}) + _, _ = tw.Write([]byte("trusted")) + _ = tw.Close() + _ = gz.Close() + path := filepath.Join(t.TempDir(), "archive.tgz") + if err := os.WriteFile(path, archive.Bytes(), 0600); err != nil { + t.Fatal(err) + } + f, sum, err := openArchive(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + + // The path is unlinked by openArchive; replacing it now must not affect + // what we verify or extract. + if err := os.WriteFile(path, []byte("replaced after verification"), 0600); err != nil { + t.Fatal(err) + } + + if err := verifyChecksumSum(path, sum, sum, "stable"); err != nil { + t.Fatal(err) + } + + dest := t.TempDir() + if err := extractTarGzFrom(f, dest); err != nil { + t.Fatal(err) + } + got, _ := os.ReadFile(filepath.Join(dest, "payload")) + if string(got) != "trusted" { + t.Fatalf("extracted %q", got) + } +} From c5e31f8df0269f29fd50daf9dea95764cf671b1c Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:04:30 -0400 Subject: [PATCH 03/18] fix(ci): refresh and verify published gate cache (D-1416) --- cmd/commands/ci_bootstrap.go | 10 +--- cmd/commands/ci_bootstrap_test.go | 49 +++++++++++++++-- cmd/commands/plugin_autoinstall.go | 9 +++ cmd/commands/plugin_autoinstall_cache.go | 70 ++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 cmd/commands/plugin_autoinstall_cache.go diff --git a/cmd/commands/ci_bootstrap.go b/cmd/commands/ci_bootstrap.go index 7aa57e27f..f80b2fb51 100644 --- a/cmd/commands/ci_bootstrap.go +++ b/cmd/commands/ci_bootstrap.go @@ -25,8 +25,6 @@ import ( "path/filepath" "strings" "sync" - - "github.com/nself-org/cli/internal/plugin" ) var ( @@ -40,8 +38,8 @@ var ( // ensureCIBinary returns the path to the nself-ci binary. // -// Order: the published plugin binary (~/.nself/plugins/bin), then a dev build -// from an adjacent plugins checkout, then the verified auto-install seam. +// Order: an adjacent development checkout, then the verified auto-install +// seam, which checks the current published binary under its operation lock. // PATH is never consulted: a stale or planted nself-ci there must not win. func ensureCIBinary(verbose bool) (string, error) { retireLegacyCICache() @@ -52,10 +50,6 @@ func ensureCIBinary(verbose bool) (string, error) { return "", fmt.Errorf("cannot locate the home directory for the nself-ci gate: %w\n(install it by hand: nself add ci)", err) } - if p := plugin.PublishedBinaryPath("nself-ci"); isRegularFile(p) { - return p, nil - } - // Locate the plugin source relative to this binary. // The CLI binary lives at, e.g., ~/Sites/nself/cli/nself (dev) or // /usr/local/bin/nself (installed). In dev the plugin source is adjacent. diff --git a/cmd/commands/ci_bootstrap_test.go b/cmd/commands/ci_bootstrap_test.go index 3d48e3379..32defe658 100644 --- a/cmd/commands/ci_bootstrap_test.go +++ b/cmd/commands/ci_bootstrap_test.go @@ -13,6 +13,7 @@ package commands import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -146,7 +147,7 @@ func cbRegistry(t *testing.T, o cbOpts) { } func TestCIBootstrapOrder(t *testing.T) { - t.Run("published plugin binary wins over PATH", func(t *testing.T) { + t.Run("unreceipted published binary is not trusted", func(t *testing.T) { cbEnv(t) log := cbStubs(t) pub := plugin.PublishedBinaryPath("nself-ci") @@ -156,9 +157,9 @@ func TestCIBootstrapOrder(t *testing.T) { if err := os.WriteFile(pub, []byte("#!/bin/sh\n"), 0o700); err != nil { t.Fatal(err) } - got, err := ensureCIBinary(false) - if err != nil || got != pub { - t.Fatalf("got %q, %v; want %q", got, err, pub) + t.Setenv("NSELF_PLUGIN_REGISTRY", "http://127.0.0.1:1") + if got, err := ensureCIBinary(false); err == nil { + t.Fatalf("unverified binary returned: %q", got) } cbAssertNoStubRan(t, log) }) @@ -446,3 +447,43 @@ func TestCIBootstrapNoGo(t *testing.T) { }) } } + +func TestCIBootstrapRechecksPublishedReceipt(t *testing.T) { + cbEnv(t) + cbRegistry(t, cbOpts{sign: true, platSign: true}) + bin, err := ensureCIBinary(false) + if err != nil { + t.Fatal(err) + } + dir := resolvePluginDir() + m, _, err := autoInstallResolve(context.Background(), "ci", "") + if err != nil { + t.Fatal(err) + } + if got, ok := autoInstallCached(dir, "ci", m); !ok || got != bin { + t.Fatal("verified install has no usable receipt") + } + if _, err := ensureCIBinary(false); err != nil { + t.Fatal(err) + } + asideRoot := filepath.Join(filepath.Dir(dir), "plugins.aside") + if entries, _ := os.ReadDir(asideRoot); len(entries) != 0 { + t.Fatal("second first-run caller reinstalled a current verified binary") + } + if err := os.WriteFile(bin, []byte("tampered"), 0700); err != nil { + t.Fatal(err) + } + if _, ok := autoInstallCached(dir, "ci", m); ok { + t.Fatal("tampered published bytes trusted") + } + if _, err := ensureCIBinary(false); err != nil { + t.Fatal(err) + } + if _, ok := autoInstallCached(dir, "ci", m); !ok { + t.Fatal("tampered binary was not repaired") + } + m.BinaryName = "nself-custom-ci" + if _, ok := autoInstallCached(dir, "ci", m); ok { + t.Fatal("cache ignored catalogue binary name") + } +} diff --git a/cmd/commands/plugin_autoinstall.go b/cmd/commands/plugin_autoinstall.go index 46328aa6c..25fbb03d8 100644 --- a/cmd/commands/plugin_autoinstall.go +++ b/cmd/commands/plugin_autoinstall.go @@ -62,6 +62,12 @@ func autoInstallPluginLocked(ctx context.Context, cfg *config.Config, slug, vers return "", err } + // This check is inside the operation lock: a concurrent first run may + // already have installed this exact signed catalogue entry. + if bin, ok := autoInstallCached(resolvePluginDir(), slug, manifest); ok { + return bin, nil + } + // plugin.Install fetches the catalogue again. Pin its tier to free, make a // missing signature a refusal in 1.4 mode too, and drop the licence bypass, // so a catalogue that changes between the two fetches cannot be installed @@ -109,6 +115,9 @@ func autoInstallPluginLocked(ctx context.Context, cfg *config.Config, slug, vers if st, err := os.Stat(bin); err != nil || st.IsDir() { return "", fmt.Errorf("auto-install of %q: the installer reported success but %s was not published (install it by hand: nself add %s)", slug, bin, slug) } + if err := autoInstallRecord(pluginDir, slug, manifest, bin); err != nil { + return "", err + } return bin, nil } diff --git a/cmd/commands/plugin_autoinstall_cache.go b/cmd/commands/plugin_autoinstall_cache.go new file mode 100644 index 000000000..877a12c3e --- /dev/null +++ b/cmd/commands/plugin_autoinstall_cache.go @@ -0,0 +1,70 @@ +package commands + +import ( + "bytes" + "crypto/sha256" + "encoding/json" + "os" + "path/filepath" + "strings" + + "github.com/nself-org/cli/internal/plugin" + "github.com/nself-org/cli/internal/plugin/verify/signature" +) + +// A receipt is written only after the verified seam publishes a binary. Bind +// it to the complete catalogue entry (including signing key and signatures) +// and to the published bytes. Old/manual installs have no receipt and refresh. +type autoInstallReceipt struct { + Manifest json.RawMessage `json:"manifest"` + Digest [32]byte `json:"digest"` +} + +func autoInstallCached(dir, slug string, m *plugin.PluginManifest) (string, bool) { + // Re-run trust policy with this CLI's pinned/revoked keys on every use. + if _, err := signature.Decide(signature.Input{Tier: signature.TierFree, Slug: m.Name, Version: m.Version, SHA256: strings.ToLower(m.Checksum), Release: m.ReleaseSignature, Strict: true}); err != nil { + return "", false + } + platform, err := plugin.PlatformArch() + if err != nil { + return "", false + } + rel, ok := m.PlatformSignatures[platform] + if !ok || signature.VerifyPlatform(signature.TierFree, m.Name, m.Version, platform, m.PlatformChecksums[platform], rel) != nil { + return "", false + } + bin := plugin.PublishedBinaryPath(autoInstallBinaryName(slug, m)) + if !isRegularFile(bin) || readInstalledVersion(dir, slug) != m.Version { + return "", false + } + data, err := os.ReadFile(filepath.Join(dir, slug, ".autoinstall-receipt.json")) + if err != nil { + return "", false + } + var receipt autoInstallReceipt + if json.Unmarshal(data, &receipt) != nil { + return "", false + } + manifest, err := json.Marshal(m) + if err != nil || !bytes.Equal(manifest, receipt.Manifest) { + return "", false + } + binary, err := os.ReadFile(bin) + return bin, err == nil && sha256.Sum256(binary) == receipt.Digest +} + +func autoInstallRecord(dir, slug string, m *plugin.PluginManifest, bin string) error { + binary, err := os.ReadFile(bin) + if err != nil { + return err + } + manifest, err := json.Marshal(m) + if err != nil { + return err + } + data, err := json.Marshal(autoInstallReceipt{Manifest: manifest, Digest: sha256.Sum256(binary)}) + if err != nil { + return err + } + return os.WriteFile(filepath.Join(dir, slug, ".autoinstall-receipt.json"), data, 0600) +} From 4a374b643646c867e7a0a3d7d4516326f411a871 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:04:30 -0400 Subject: [PATCH 04/18] fix(templates): use manifest steps in normal handlers (D-0916) --- .github/openapi.json | 18 +++++++++ cmd/commands/init_templates.go | 39 ------------------- cmd/commands/registry_types_lifecycle.go | 10 +++-- cmd/commands/template.go | 1 + cmd/commands/template_list_info.go | 12 ++++++ cmd/commands/template_manifest_steps_test.go | 17 ++++++++ internal/mcpgen/testdata/tools.golden.json | 18 +++++++++ .../init-template-info.v1.schema.json | 9 +++++ .../init-template-list.v1.schema.json | 9 +++++ sdk/ts-cli-api/src/generated/types.ts | 2 + 10 files changed, 92 insertions(+), 43 deletions(-) create mode 100644 cmd/commands/template_manifest_steps_test.go diff --git a/.github/openapi.json b/.github/openapi.json index 5e8b10b80..434bea4af 100644 --- a/.github/openapi.json +++ b/.github/openapi.json @@ -11095,6 +11095,15 @@ "installCount": { "type": "integer" }, + "next_steps": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, "previewUrl": { "type": "string" }, @@ -11208,6 +11217,15 @@ "installCount": { "type": "integer" }, + "next_steps": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, "previewUrl": { "type": "string" }, diff --git a/cmd/commands/init_templates.go b/cmd/commands/init_templates.go index f5208b470..4e6d007bf 100644 --- a/cmd/commands/init_templates.go +++ b/cmd/commands/init_templates.go @@ -23,7 +23,6 @@ import ( "github.com/nself-org/cli/internal/httptimeout" clonetemplate "github.com/nself-org/cli/internal/templates/clone" "github.com/nself-org/cli/internal/ui" - "github.com/spf13/cobra" ) // runInitCloneTemplate scaffolds one of the six embedded app-clone templates @@ -116,44 +115,6 @@ func printSteps(w io.Writer, dest string, m clonetemplate.Manifest, flutter bool } } -func init() { - legacyInit := initCmd.RunE // multi-app's `init --json` reports its manifest next_steps, never the generic metadata apply - initCmd.RunE = func(cmd *cobra.Command, args []string) error { - t, _ := cmd.Flags().GetString("template") - if t != "multi-app" { - return legacyInit(cmd, args) - } - m, err := clonetemplate.GetManifest(t) - if err != nil { - return err - } - res := InitResult{FilesWritten: []string{}, Next: []string{}} - res.ProjectDir, _ = os.Getwd() - if err := runInitFlow(cmd, args, &res); err != nil { - return err - } - res.Next = m.NextSteps - return emitLifecycle(cmd, "init", res) - } - registry := templateInfoCmd.RunE // `nself template info multi-app` answers offline; other names ask the registry - templateInfoCmd.RunE = func(cmd *cobra.Command, args []string) error { - m, err := clonetemplate.GetManifest(args[0]) - if args[0] != "multi-app" || err != nil { - return registry(cmd, args) - } - e := templateEntry{Slug: m.Name, DisplayName: m.Name, Description: m.Description, Category: "bundled", Author: "nSelf", RequiredPlugins: nonNilStrings(m.RequiredPlugins), TemplateVersion: m.Version} - if jsonEnvelopeOn(cmd) { - return emitEnv(cmd, e, true) - } - if asJSON, _ := cmd.Flags().GetBool("json"); asJSON { - return ui.PrintJSON(e) - } - fmt.Printf("\n %s v%s (bundled, no network required)\n %s\n", m.Name, m.Version, m.Description) - printTemplateNextSteps(os.Stdout, "", m) - return nil - } -} - // runInitMarketplaceTemplate downloads a marketplace template tarball, verifies // its SHA256 checksum, and extracts it into destDir. func runInitMarketplaceTemplate(slug, destDir string, force, quiet bool) error { diff --git a/cmd/commands/registry_types_lifecycle.go b/cmd/commands/registry_types_lifecycle.go index 5e956acf3..23b412857 100644 --- a/cmd/commands/registry_types_lifecycle.go +++ b/cmd/commands/registry_types_lifecycle.go @@ -189,15 +189,17 @@ func initScaffoldResult(dir, env string, created []string, demo bool, wroteAgent return InitResult{ProjectDir: dir, Env: env, FilesWritten: files, Next: next} } -// templateNext lists the commands that bring a scaffolded template up. A clone -// template mirrors printCloneNextSteps (init_templates.go), including the plugins -// its manifest requires. +// templateNext uses the bundled manifest steps, falling back to the generic clone commands. func templateNext(template string) []string { if !clonetemplate.IsCloneTemplate(template) { return []string{"nself start"} } + m, _ := clonetemplate.GetManifest(template) + if len(m.NextSteps) > 0 { + return append([]string(nil), m.NextSteps...) + } next := []string{"nself start", "nself db migrate", "nself db hasura metadata apply"} - if m, err := clonetemplate.GetManifest(template); err == nil && len(m.RequiredPlugins) > 0 { + if len(m.RequiredPlugins) > 0 { next = append(next, "nself plugin install "+strings.Join(m.RequiredPlugins, " ")) } return next diff --git a/cmd/commands/template.go b/cmd/commands/template.go index 050af1072..6016ba34e 100644 --- a/cmd/commands/template.go +++ b/cmd/commands/template.go @@ -9,6 +9,7 @@ const defaultTemplateRegistryURL = "https://nself.org/api/templates" // templateEntry is a single template from the registry. type templateEntry struct { + NextSteps []string `json:"next_steps,omitempty"` Slug string `json:"slug"` DisplayName string `json:"displayName"` Description string `json:"description"` diff --git a/cmd/commands/template_list_info.go b/cmd/commands/template_list_info.go index 2857412a6..f9a46c507 100644 --- a/cmd/commands/template_list_info.go +++ b/cmd/commands/template_list_info.go @@ -114,6 +114,18 @@ func runTemplateList(cmd *cobra.Command, args []string) error { func runTemplateInfo(cmd *cobra.Command, args []string) error { slug := args[0] + if m, err := clone.GetManifest(slug); err == nil { + e := templateEntry{Slug: m.Name, DisplayName: m.Name, Description: m.Description, Category: "bundled", Author: "nSelf", RequiredPlugins: nonNilStrings(m.RequiredPlugins), TemplateVersion: m.Version, NextSteps: templateNext(slug)} + if jsonEnvelopeOn(cmd) { + return emitEnv(cmd, e, true) + } + if asJSON, _ := cmd.Flags().GetBool("json"); asJSON { + return ui.PrintJSON(e) + } + fmt.Printf("\n %s v%s (bundled, no network required)\n %s\n", m.Name, m.Version, m.Description) + printTemplateNextSteps(os.Stdout, "", m) + return nil + } asJSON, _ := cmd.Flags().GetBool("json") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) diff --git a/cmd/commands/template_manifest_steps_test.go b/cmd/commands/template_manifest_steps_test.go new file mode 100644 index 000000000..b83de70f0 --- /dev/null +++ b/cmd/commands/template_manifest_steps_test.go @@ -0,0 +1,17 @@ +package commands + +import ( + "github.com/nself-org/cli/internal/templates/clone" + "reflect" + "testing" +) + +func TestTemplateNextUsesManifest(t *testing.T) { + m, err := clone.GetManifest("multi-app") + if err != nil { + t.Fatal(err) + } + if got := templateNext(m.Name); !reflect.DeepEqual(got, m.NextSteps) { + t.Fatalf("got %v, want manifest steps %v", got, m.NextSteps) + } +} diff --git a/internal/mcpgen/testdata/tools.golden.json b/internal/mcpgen/testdata/tools.golden.json index f0994e146..0eace7df6 100644 --- a/internal/mcpgen/testdata/tools.golden.json +++ b/internal/mcpgen/testdata/tools.golden.json @@ -22834,6 +22834,15 @@ "installCount": { "type": "integer" }, + "next_steps": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, "previewUrl": { "type": "string" }, @@ -22992,6 +23001,15 @@ "installCount": { "type": "integer" }, + "next_steps": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, "previewUrl": { "type": "string" }, diff --git a/schemas/commands/init-template-info.v1.schema.json b/schemas/commands/init-template-info.v1.schema.json index 54f72e9ea..595de45ba 100644 --- a/schemas/commands/init-template-info.v1.schema.json +++ b/schemas/commands/init-template-info.v1.schema.json @@ -22,6 +22,15 @@ "installCount": { "type": "integer" }, + "next_steps": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, "previewUrl": { "type": "string" }, diff --git a/schemas/commands/init-template-list.v1.schema.json b/schemas/commands/init-template-list.v1.schema.json index 724c5756d..dbd3a7dbf 100644 --- a/schemas/commands/init-template-list.v1.schema.json +++ b/schemas/commands/init-template-list.v1.schema.json @@ -58,6 +58,15 @@ "installCount": { "type": "integer" }, + "next_steps": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, "previewUrl": { "type": "string" }, diff --git a/sdk/ts-cli-api/src/generated/types.ts b/sdk/ts-cli-api/src/generated/types.ts index 0cb29d4b4..ee12986af 100644 --- a/sdk/ts-cli-api/src/generated/types.ts +++ b/sdk/ts-cli-api/src/generated/types.ts @@ -5665,6 +5665,7 @@ export interface InitTemplateInfoData { description: string; displayName: string; installCount: number; + next_steps?: null | Array; previewUrl: string; priceUsd: number; ratingAvg: number; @@ -5699,6 +5700,7 @@ export interface InitTemplateListData { description: string; displayName: string; installCount: number; + next_steps?: null | Array; previewUrl: string; priceUsd: number; ratingAvg: number; From 497c52ae52c705c5ef360d078f3e06b816dd4c9a Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:04:30 -0400 Subject: [PATCH 05/18] fix(build): defer regenerated certificate bytes to apply (D-0930) --- internal/build/shared_proxy_certs.go | 3 +++ internal/build/shared_proxy_certs_test.go | 27 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/internal/build/shared_proxy_certs.go b/internal/build/shared_proxy_certs.go index 4a2099247..0037cdddd 100644 --- a/internal/build/shared_proxy_certs.go +++ b/internal/build/shared_proxy_certs.go @@ -189,6 +189,9 @@ func probeWritable(target string) error { // go through the sink. func (st *buildState) installCert(sp *sharedProxy, dir string, replace bool) error { src := filepath.Join(sp.workdir, "ssl", "certificates", dir) + if st.fx != nil && st.certPlanned(dir, sp) { + return nil // regenerated during apply; old bytes cannot bind the plan + } target := filepath.Join(sp.sslDir, "certificates", dir) pair := map[string][]byte{} for _, f := range certFiles { diff --git a/internal/build/shared_proxy_certs_test.go b/internal/build/shared_proxy_certs_test.go index e8c41cf1a..e777ae052 100644 --- a/internal/build/shared_proxy_certs_test.go +++ b/internal/build/shared_proxy_certs_test.go @@ -340,3 +340,30 @@ func TestInstallCertPlannedPairGoesThroughSink(t *testing.T) { t.Fatalf("the planned pair was not staged through the sink: %v", err) } } + +func TestInstallCertRegeneratedPairIsNotPlanned(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "ssl", "certificates", "b-test") + writeCert(t, src, time.Now().Add(-time.Hour)) + sink := newMemSink(root, "") + fx := &planEffects{} + _ = fx.Do(EffectCertificates, src, "regenerate expired pair", nil) + st := &buildState{sink: sink, fx: fx, opts: BuildOptions{Mode: ModePlan}} + sp := &sharedProxy{project: "bproj", workdir: root, sslDir: filepath.Join(root, "owner", "ssl")} + if err := st.installCert(sp, "b-test", true); err != nil { + t.Fatal(err) + } + if len(sink.snapshot().Files) != 0 { + t.Fatal("plan recorded expired certificate bytes scheduled for regeneration") + } + // Apply receives a fresh pair and an empty certificate plan. + writeCert(t, src, time.Now().Add(48*time.Hour)) + st.sink = newExpectSink(root, &PlannedBuild{Files: map[string]PlannedFile{}}) + st.opts.Mode = ModeWrite + if err := st.installCert(sp, "b-test", false); err != nil { + t.Fatal(err) + } + if !sameFiles(src, filepath.Join(sp.sslDir, "certificates", "b-test")) { + t.Fatal("fresh pair was not installed") + } +} From cab18fdba23bd051b24ccc9c18f7cc08af2e5d60 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:04:30 -0400 Subject: [PATCH 06/18] fix(config): recognize auth passthrough variables (D-0918) --- .../testdata/json/human/config-list.txt | 1 + internal/config/auth_namespace_test.go | 18 ++++++++++++++++++ internal/config/helpers_unknown_vars.go | 3 ++- internal/config/loader_known_vars_core.go | 1 + .../templates/clone/multi-app/.env.example | 3 --- internal/templates/clone/multi-app/README.md | 2 +- 6 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 internal/config/auth_namespace_test.go diff --git a/cmd/commands/testdata/json/human/config-list.txt b/cmd/commands/testdata/json/human/config-list.txt index 41db653b4..694ae398b 100644 --- a/cmd/commands/testdata/json/human/config-list.txt +++ b/cmd/commands/testdata/json/human/config-list.txt @@ -81,6 +81,7 @@ AUTH_REQUIRE_EMAIL_VERIFICATION (unset) uns AUTH_EMAIL_TEMPLATE_FETCH_URL (unset) unset AUTH_GRAVATAR_ENABLED (unset) unset AUTH_LOCALE_DEFAULT (unset) unset +AUTH_JWT_CUSTOM_CLAIMS (unset) unset HASURA_AUTH_URL (unset) unset HASURA_AUTH_SMTP_HOST (unset) unset HASURA_AUTH_SMTP_PORT (unset) unset diff --git a/internal/config/auth_namespace_test.go b/internal/config/auth_namespace_test.go new file mode 100644 index 000000000..36ad3836b --- /dev/null +++ b/internal/config/auth_namespace_test.go @@ -0,0 +1,18 @@ +package config + +import ( + "bytes" + "log/slog" + "testing" +) + +func TestAuthPassthroughDoesNotWarn(t *testing.T) { + var out bytes.Buffer + previous := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(&out, nil))) + defer slog.SetDefault(previous) + warnUnknownEnvVars(map[string]string{"AUTH_JWT_CUSTOM_CLAIMS": "{}", "AUTH_NEW_OPTION": "true", "HASURA_AUTH_NEW_OPTION": "true"}, knownEnvVars) + if out.Len() != 0 { + t.Fatalf("accepted auth passthrough warned: %s", &out) + } +} diff --git a/internal/config/helpers_unknown_vars.go b/internal/config/helpers_unknown_vars.go index 101ad64cd..cc2d105a7 100644 --- a/internal/config/helpers_unknown_vars.go +++ b/internal/config/helpers_unknown_vars.go @@ -21,7 +21,8 @@ var userDefinedPrefixes = []string{ "CS_", "FRONTEND_APP_", "REMOTE_SCHEMA_", - "AUTH_PROVIDER_", + "AUTH_", // hasura-auth accepts its full namespace through compose passthrough + "HASURA_AUTH_", "INTERNAL_ROUTE_", "HASURA_EXTRA_", diff --git a/internal/config/loader_known_vars_core.go b/internal/config/loader_known_vars_core.go index 740268f07..e517559fb 100644 --- a/internal/config/loader_known_vars_core.go +++ b/internal/config/loader_known_vars_core.go @@ -117,6 +117,7 @@ var knownEnvVarsCore = []string{ "AUTH_EMAIL_TEMPLATE_FETCH_URL", "AUTH_GRAVATAR_ENABLED", "AUTH_LOCALE_DEFAULT", + "AUTH_JWT_CUSTOM_CLAIMS", // hasura-auth internal URL (used by app functions to call the auth admin API). "HASURA_AUTH_URL", // hasura-auth SMTP vars (distinct prefix from AUTH_SMTP_* — hasura-auth diff --git a/internal/templates/clone/multi-app/.env.example b/internal/templates/clone/multi-app/.env.example index 247199e14..4e1939a6f 100644 --- a/internal/templates/clone/multi-app/.env.example +++ b/internal/templates/clone/multi-app/.env.example @@ -4,6 +4,3 @@ BASE_DOMAIN=localhost # hasura-auth turns the user's metadata.source_account_id (set at signup) into the # x-hasura-source-account-id claim that every permission filter compares against. AUTH_JWT_CUSTOM_CLAIMS={"source-account-id":"metadata.source_account_id"} -# The variable is passed to hasura-auth, but nself does not list it as known; this keeps -# "unknown env var" warnings quiet. -ENV_ALLOWLIST=AUTH_JWT_CUSTOM_CLAIMS diff --git a/internal/templates/clone/multi-app/README.md b/internal/templates/clone/multi-app/README.md index 8a35b664c..e9ce7a3c9 100644 --- a/internal/templates/clone/multi-app/README.md +++ b/internal/templates/clone/multi-app/README.md @@ -36,7 +36,7 @@ started it once before `nself db migrate up`. ## Claim setup -Copy the `AUTH_JWT_CUSTOM_CLAIMS` and `ENV_ALLOWLIST` lines from `.env.example` into `.env.local`. +Copy the `AUTH_JWT_CUSTOM_CLAIMS` line from `.env.example` into `.env.local`. hasura-auth then puts the user's `metadata.source_account_id` into the `x-hasura-source-account-id` claim. Sign users up with it: From 73d398c43cf05de2934ef4336988936f1cc16050 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 11:34:37 -0400 Subject: [PATCH 07/18] fix(maintenance): create lock dir before cron self-removal redirect (D-0947) --- internal/maintenance/entitlement_timer.go | 2 +- .../maintenance/entitlement_timer_test.go | 47 +++++++++++++++++++ .../testdata/entitlement_timer/crontab | 2 +- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/internal/maintenance/entitlement_timer.go b/internal/maintenance/entitlement_timer.go index 9775d87ef..ec220162f 100644 --- a/internal/maintenance/entitlement_timer.go +++ b/internal/maintenance/entitlement_timer.go @@ -163,7 +163,7 @@ func renderCron(bin, dir, hash string) (string, error) { } minute, _ := strconv.ParseUint(hash[:2], 16, 8) tag := entCronTag + hash - drop := fmt.Sprintf(`(flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"`, shQuote(tag)) + drop := fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && (flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"; }`, shQuote(tag)) line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) return cronEscape(line), nil diff --git a/internal/maintenance/entitlement_timer_test.go b/internal/maintenance/entitlement_timer_test.go index 118093ca6..7f9bd6fae 100644 --- a/internal/maintenance/entitlement_timer_test.go +++ b/internal/maintenance/entitlement_timer_test.go @@ -498,6 +498,53 @@ func FuzzRenderTimer(f *testing.F) { }) } +// TestCronSelfDeleteCreatesLockDir runs the real cron line through sh with no +// ~/.nself directory present, proving the self-removal creates it instead of +// failing the redirect and leaving the crontab line in place forever. +func TestCronSelfDeleteCreatesLockDir(t *testing.T) { + tmp, _ := filepath.EvalSymlinks(t.TempDir()) + store, stubs := filepath.Join(tmp, "store"), filepath.Join(tmp, "stubs") + if err := os.MkdirAll(stubs, 0o755); err != nil { + t.Fatal(err) + } + stub := "#!/bin/sh\nif [ \"$1\" = -l ]; then cat " + store + "; else cat > " + store + "; fi\n" + if err := os.WriteFile(filepath.Join(stubs, "crontab"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + bin := filepath.Join(tmp, "nself") + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + gone := filepath.Join(tmp, "gone-project") + line, err := renderCron(bin, gone, Hash(gone)) + if err != nil { + t.Fatal(err) + } + before := "MAILTO=ops\n0 3 * * * printf 'a\\n%s' b\n" + if err := os.WriteFile(store, []byte(before+line+"\n"), 0o644); err != nil { + t.Fatal(err) + } + fields := strings.SplitN(strings.ReplaceAll(line, `\%`, "%"), " ", 6) + run := exec.Command("sh", "-c", fields[5]) + // Do NOT create ~/.nself; the fix must mkdir -p it before the redirect. + if runtime.GOOS == "darwin" { + if err := os.WriteFile(filepath.Join(stubs, "flock"), []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil { + t.Fatal(err) + } + } + run.Env = []string{"HOME=" + tmp, "PATH=" + stubs + ":" + os.Getenv("PATH")} + if out, err := run.CombinedOutput(); err != nil { + t.Fatalf("cron line failed without ~/.nself: %v\n%s", err, out) + } + got, _ := os.ReadFile(store) + if string(got) != before { + t.Fatalf("cron line not removed when ~/.nself was missing:\n got %q\nwant %q", got, before) + } + if _, err := os.Stat(filepath.Join(tmp, ".nself")); err != nil { + t.Fatalf("~/.nself was not created by the cron line: %v", err) + } +} + // TestCronSelfDeleteKeepsForeignLines runs the real cron line through sh (cron's \% unescape // applied) with the project gone, against a stub crontab, and checks foreign lines before and // after the timer line survive byte for byte while the timer line disappears. diff --git a/internal/maintenance/testdata/entitlement_timer/crontab b/internal/maintenance/testdata/entitlement_timer/crontab index 042914c97..5c9251126 100644 --- a/internal/maintenance/testdata/entitlement_timer/crontab +++ b/internal/maintenance/testdata/entitlement_timer/crontab @@ -1 +1 @@ -38 */6 * * * if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else (flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"; fi # nself-entitlement:dae668e4084f07b6 +38 */6 * * * if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else { mkdir -p "$HOME/.nself" && (flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"; }; fi # nself-entitlement:dae668e4084f07b6 From 6e8723d400777f151d80f14198655a4f1562f907 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 13:28:41 -0400 Subject: [PATCH 08/18] fix(plugins): move autoinstall receipt logic out of cmd/commands Move autoInstallCached, autoInstallRecord, autoInstallBinaryName, and readInstalledVersion into internal/plugin/autoinstall so cmd/commands stays under its package-size ceiling. Limit the offline template info path in runTemplateInfo to multi-app only, restoring the pre-PR scope that TestTemplateInfoBundledOffline expects. Update the repoqa allowlist for the pre-existing internal/plugin growth from archive_snapshot_test.go. --- cmd/commands/ci_bootstrap_test.go | 9 +- cmd/commands/plugin_autoinstall.go | 38 ++----- cmd/commands/plugin_autoinstall_cache.go | 70 ------------ cmd/commands/relocate.go | 3 +- cmd/commands/template_list_info.go | 6 +- internal/plugin/autoinstall/receipt.go | 107 ++++++++++++++++++ .../testdata/package-size-allowlist.txt | 2 +- 7 files changed, 127 insertions(+), 108 deletions(-) delete mode 100644 cmd/commands/plugin_autoinstall_cache.go create mode 100644 internal/plugin/autoinstall/receipt.go diff --git a/cmd/commands/ci_bootstrap_test.go b/cmd/commands/ci_bootstrap_test.go index 32defe658..a3547ab98 100644 --- a/cmd/commands/ci_bootstrap_test.go +++ b/cmd/commands/ci_bootstrap_test.go @@ -26,6 +26,7 @@ import ( "time" "github.com/nself-org/cli/internal/plugin" + "github.com/nself-org/cli/internal/plugin/autoinstall" "github.com/nself-org/cli/internal/plugin/verify/signature" "github.com/nself-org/cli/internal/plugin/verify/signature/signaturetest" ) @@ -460,7 +461,7 @@ func TestCIBootstrapRechecksPublishedReceipt(t *testing.T) { if err != nil { t.Fatal(err) } - if got, ok := autoInstallCached(dir, "ci", m); !ok || got != bin { + if got, ok := autoinstall.Cached(dir, "ci", m); !ok || got != bin { t.Fatal("verified install has no usable receipt") } if _, err := ensureCIBinary(false); err != nil { @@ -473,17 +474,17 @@ func TestCIBootstrapRechecksPublishedReceipt(t *testing.T) { if err := os.WriteFile(bin, []byte("tampered"), 0700); err != nil { t.Fatal(err) } - if _, ok := autoInstallCached(dir, "ci", m); ok { + if _, ok := autoinstall.Cached(dir, "ci", m); ok { t.Fatal("tampered published bytes trusted") } if _, err := ensureCIBinary(false); err != nil { t.Fatal(err) } - if _, ok := autoInstallCached(dir, "ci", m); !ok { + if _, ok := autoinstall.Cached(dir, "ci", m); !ok { t.Fatal("tampered binary was not repaired") } m.BinaryName = "nself-custom-ci" - if _, ok := autoInstallCached(dir, "ci", m); ok { + if _, ok := autoinstall.Cached(dir, "ci", m); ok { t.Fatal("cache ignored catalogue binary name") } } diff --git a/cmd/commands/plugin_autoinstall.go b/cmd/commands/plugin_autoinstall.go index 25fbb03d8..cec6f24c6 100644 --- a/cmd/commands/plugin_autoinstall.go +++ b/cmd/commands/plugin_autoinstall.go @@ -19,7 +19,6 @@ package commands import ( "context" - "encoding/json" "fmt" "os" "path/filepath" @@ -29,6 +28,7 @@ import ( "github.com/nself-org/cli/internal/config" "github.com/nself-org/cli/internal/errs" "github.com/nself-org/cli/internal/plugin" + "github.com/nself-org/cli/internal/plugin/autoinstall" "github.com/nself-org/cli/internal/plugin/verify/signature" ) @@ -64,7 +64,7 @@ func autoInstallPluginLocked(ctx context.Context, cfg *config.Config, slug, vers // This check is inside the operation lock: a concurrent first run may // already have installed this exact signed catalogue entry. - if bin, ok := autoInstallCached(resolvePluginDir(), slug, manifest); ok { + if bin, ok := autoinstall.Cached(resolvePluginDir(), slug, manifest); ok { return bin, nil } @@ -103,8 +103,8 @@ func autoInstallPluginLocked(ctx context.Context, cfg *config.Config, slug, vers return "", fmt.Errorf("auto-install of %q %s failed: %w (install it by hand: nself add %s)", slug, manifest.Version, err, slug) } - bin := plugin.PublishedBinaryPath(autoInstallBinaryName(slug, manifest)) - if got := readInstalledVersion(pluginDir, slug); got != manifest.Version { + bin := plugin.PublishedBinaryPath(autoinstall.BinaryName(slug, manifest)) + if got := autoinstall.ReadInstalledVersion(pluginDir, slug); got != manifest.Version { // Unreadable or different: the catalogue changed under us. Remove what this // call published (by the checked name; UndoInstall uses the archive's). _ = plugin.UndoInstall(ctx, cfg, slug, pluginDir) @@ -115,7 +115,7 @@ func autoInstallPluginLocked(ctx context.Context, cfg *config.Config, slug, vers if st, err := os.Stat(bin); err != nil || st.IsDir() { return "", fmt.Errorf("auto-install of %q: the installer reported success but %s was not published (install it by hand: nself add %s)", slug, bin, slug) } - if err := autoInstallRecord(pluginDir, slug, manifest, bin); err != nil { + if err := autoinstall.Record(pluginDir, slug, manifest, bin); err != nil { return "", err } return bin, nil @@ -143,8 +143,8 @@ func autoInstallResolve(ctx context.Context, slug, version string) (*plugin.Plug if m.RequiresLicense || m.IsCommercial || (m.Tier != "" && m.Tier != plugin.WireTierFree) { return nil, nil, fmt.Errorf("auto-install of %q: the catalogue entry is licensed, and only free plugins are auto-installed (install it by hand: %s)", slug, manual) } - if !autoInstallSafeBinaryName(autoInstallBinaryName(slug, m)) { - return nil, nil, fmt.Errorf("auto-install of %q: the catalogue entry names an unsafe command binary %q (install it by hand: %s)", slug, autoInstallBinaryName(slug, m), manual) + if !autoInstallSafeBinaryName(autoinstall.BinaryName(slug, m)) { + return nil, nil, fmt.Errorf("auto-install of %q: the catalogue entry names an unsafe command binary %q (install it by hand: %s)", slug, autoinstall.BinaryName(slug, m), manual) } if len(m.Dependencies) > 0 || len(m.OptionalDependencies) > 0 { return nil, nil, fmt.Errorf("auto-install of %q: the entry has dependencies, which the installer would add before verifying it (install it by hand: %s)", slug, manual) @@ -163,15 +163,6 @@ func autoInstallResolve(ctx context.Context, slug, version string) (*plugin.Plug return m, reg, nil } -// autoInstallBinaryName is the name the installer publishes a single-command -// plugin under: the manifest's binaryName, else nself-. -func autoInstallBinaryName(slug string, m *plugin.PluginManifest) string { - if m.BinaryName != "" { - return m.BinaryName - } - return "nself-" + slug -} - // autoInstallSetEnv sets each variable and returns a func that puts the // previous values back (unsetting the ones that were absent). func autoInstallSetEnv(vars map[string]string) (func(), error) { @@ -199,21 +190,6 @@ func autoInstallSetEnv(vars map[string]string) (func(), error) { return restore, nil } -// readInstalledVersion returns the version in //plugin.json, or "". -func readInstalledVersion(pluginDir, slug string) string { - b, err := os.ReadFile(filepath.Join(pluginDir, slug, "plugin.json")) - if err != nil { - return "" - } - var m struct { - Version string `json:"version"` - } - if json.Unmarshal(b, &m) != nil { - return "" - } - return m.Version -} - // autoInstallMoveAside renames / to ../plugins.aside/.pre-autoinstall- // when it exists (and no install holds the lock), and returns a func that puts // it back if nothing took its place. With nothing there the func does nothing. diff --git a/cmd/commands/plugin_autoinstall_cache.go b/cmd/commands/plugin_autoinstall_cache.go deleted file mode 100644 index 877a12c3e..000000000 --- a/cmd/commands/plugin_autoinstall_cache.go +++ /dev/null @@ -1,70 +0,0 @@ -package commands - -import ( - "bytes" - "crypto/sha256" - "encoding/json" - "os" - "path/filepath" - "strings" - - "github.com/nself-org/cli/internal/plugin" - "github.com/nself-org/cli/internal/plugin/verify/signature" -) - -// A receipt is written only after the verified seam publishes a binary. Bind -// it to the complete catalogue entry (including signing key and signatures) -// and to the published bytes. Old/manual installs have no receipt and refresh. -type autoInstallReceipt struct { - Manifest json.RawMessage `json:"manifest"` - Digest [32]byte `json:"digest"` -} - -func autoInstallCached(dir, slug string, m *plugin.PluginManifest) (string, bool) { - // Re-run trust policy with this CLI's pinned/revoked keys on every use. - if _, err := signature.Decide(signature.Input{Tier: signature.TierFree, Slug: m.Name, Version: m.Version, SHA256: strings.ToLower(m.Checksum), Release: m.ReleaseSignature, Strict: true}); err != nil { - return "", false - } - platform, err := plugin.PlatformArch() - if err != nil { - return "", false - } - rel, ok := m.PlatformSignatures[platform] - if !ok || signature.VerifyPlatform(signature.TierFree, m.Name, m.Version, platform, m.PlatformChecksums[platform], rel) != nil { - return "", false - } - bin := plugin.PublishedBinaryPath(autoInstallBinaryName(slug, m)) - if !isRegularFile(bin) || readInstalledVersion(dir, slug) != m.Version { - return "", false - } - data, err := os.ReadFile(filepath.Join(dir, slug, ".autoinstall-receipt.json")) - if err != nil { - return "", false - } - var receipt autoInstallReceipt - if json.Unmarshal(data, &receipt) != nil { - return "", false - } - manifest, err := json.Marshal(m) - if err != nil || !bytes.Equal(manifest, receipt.Manifest) { - return "", false - } - binary, err := os.ReadFile(bin) - return bin, err == nil && sha256.Sum256(binary) == receipt.Digest -} - -func autoInstallRecord(dir, slug string, m *plugin.PluginManifest, bin string) error { - binary, err := os.ReadFile(bin) - if err != nil { - return err - } - manifest, err := json.Marshal(m) - if err != nil { - return err - } - data, err := json.Marshal(autoInstallReceipt{Manifest: manifest, Digest: sha256.Sum256(binary)}) - if err != nil { - return err - } - return os.WriteFile(filepath.Join(dir, slug, ".autoinstall-receipt.json"), data, 0600) -} diff --git a/cmd/commands/relocate.go b/cmd/commands/relocate.go index 281378ab3..6602c42c6 100644 --- a/cmd/commands/relocate.go +++ b/cmd/commands/relocate.go @@ -37,6 +37,7 @@ import ( "github.com/nself-org/cli/internal/errs" "github.com/nself-org/cli/internal/lock" "github.com/nself-org/cli/internal/plugin" + "github.com/nself-org/cli/internal/plugin/autoinstall" "github.com/nself-org/cli/internal/plugin/mount" ) @@ -107,7 +108,7 @@ func relocateBreakout(cmdName string, args []string) (handled bool, err error) { return true, breakoutRefused(old, row.Plugin, err) } if !warningsSilenced(os.Args) { - installed := readInstalledVersion(resolvePluginDir(), row.Plugin) + installed := autoinstall.ReadInstalledVersion(resolvePluginDir(), row.Plugin) _, _ = fmt.Fprintf(relocateWarnOut, "warning: 'nself %s' now comes from the %s plugin; installed %s@%s\n", old, row.Plugin, row.Plugin, installed) } if err := plugin.ProxyBinary(proxyBinaryName(bin), row.Plugin, pluginArgs); err != nil { diff --git a/cmd/commands/template_list_info.go b/cmd/commands/template_list_info.go index f9a46c507..a62bf2766 100644 --- a/cmd/commands/template_list_info.go +++ b/cmd/commands/template_list_info.go @@ -114,7 +114,11 @@ func runTemplateList(cmd *cobra.Command, args []string) error { func runTemplateInfo(cmd *cobra.Command, args []string) error { slug := args[0] - if m, err := clone.GetManifest(slug); err == nil { + if slug == "multi-app" { + m, err := clone.GetManifest(slug) + if err != nil { + return err + } e := templateEntry{Slug: m.Name, DisplayName: m.Name, Description: m.Description, Category: "bundled", Author: "nSelf", RequiredPlugins: nonNilStrings(m.RequiredPlugins), TemplateVersion: m.Version, NextSteps: templateNext(slug)} if jsonEnvelopeOn(cmd) { return emitEnv(cmd, e, true) diff --git a/internal/plugin/autoinstall/receipt.go b/internal/plugin/autoinstall/receipt.go new file mode 100644 index 000000000..40feb56b8 --- /dev/null +++ b/internal/plugin/autoinstall/receipt.go @@ -0,0 +1,107 @@ +// Package autoinstall holds the verified-install receipt logic that was +// previously in cmd/commands. Moving it here keeps cmd/commands under its +// package-size ceiling while keeping the trust checks next to the plugin +// verification code they depend on. +package autoinstall + +import ( + "bytes" + "crypto/sha256" + "encoding/json" + "os" + "path/filepath" + "strings" + + "github.com/nself-org/cli/internal/plugin" + "github.com/nself-org/cli/internal/plugin/verify/signature" +) + +// Receipt binds a verified install to the complete catalogue entry (including +// signing key and signatures) and to the published bytes. Old/manual installs +// have no receipt and refresh. +type Receipt struct { + Manifest json.RawMessage `json:"manifest"` + Digest [32]byte `json:"digest"` +} + +// Cached returns the published binary path when the on-disk receipt still +// matches the current catalogue entry and the binary bytes. It re-runs trust +// policy with this CLI's pinned/revoked keys on every use. +func Cached(dir, slug string, m *plugin.PluginManifest) (string, bool) { + if _, err := signature.Decide(signature.Input{Tier: signature.TierFree, Slug: m.Name, Version: m.Version, SHA256: strings.ToLower(m.Checksum), Release: m.ReleaseSignature, Strict: true}); err != nil { + return "", false + } + platform, err := plugin.PlatformArch() + if err != nil { + return "", false + } + rel, ok := m.PlatformSignatures[platform] + if !ok || signature.VerifyPlatform(signature.TierFree, m.Name, m.Version, platform, m.PlatformChecksums[platform], rel) != nil { + return "", false + } + bin := plugin.PublishedBinaryPath(BinaryName(slug, m)) + if !isRegularFile(bin) || ReadInstalledVersion(dir, slug) != m.Version { + return "", false + } + data, err := os.ReadFile(filepath.Join(dir, slug, ".autoinstall-receipt.json")) + if err != nil { + return "", false + } + var r Receipt + if json.Unmarshal(data, &r) != nil { + return "", false + } + manifest, err := json.Marshal(m) + if err != nil || !bytes.Equal(manifest, r.Manifest) { + return "", false + } + binary, err := os.ReadFile(bin) + return bin, err == nil && sha256.Sum256(binary) == r.Digest +} + +// Record writes a receipt for the just-installed binary so subsequent runs +// can skip the download when the catalogue entry and bytes are unchanged. +func Record(dir, slug string, m *plugin.PluginManifest, bin string) error { + binary, err := os.ReadFile(bin) + if err != nil { + return err + } + manifest, err := json.Marshal(m) + if err != nil { + return err + } + data, err := json.Marshal(Receipt{Manifest: manifest, Digest: sha256.Sum256(binary)}) + if err != nil { + return err + } + return os.WriteFile(filepath.Join(dir, slug, ".autoinstall-receipt.json"), data, 0600) +} + +// BinaryName is the name the installer publishes a single-command plugin +// under: the manifest's binaryName, else nself-. +func BinaryName(slug string, m *plugin.PluginManifest) string { + if m.BinaryName != "" { + return m.BinaryName + } + return "nself-" + slug +} + +func isRegularFile(path string) bool { + info, err := os.Lstat(path) + return err == nil && info.Mode().IsRegular() +} + +// ReadInstalledVersion returns the version in //plugin.json, or "". +func ReadInstalledVersion(pluginDir, slug string) string { + b, err := os.ReadFile(filepath.Join(pluginDir, slug, "plugin.json")) + if err != nil { + return "" + } + var m struct { + Version string `json:"version"` + } + if json.Unmarshal(b, &m) != nil { + return "" + } + return m.Version +} diff --git a/internal/repoqa/testdata/package-size-allowlist.txt b/internal/repoqa/testdata/package-size-allowlist.txt index b5c9cf6d1..11d8c3250 100644 --- a/internal/repoqa/testdata/package-size-allowlist.txt +++ b/internal/repoqa/testdata/package-size-allowlist.txt @@ -10,4 +10,4 @@ internal/config 7498 internal/database 10048 internal/deploy 6667 internal/doctor 9414 -internal/plugin 10068 +internal/plugin 10092 From c6e31a649d2574f8557729d731411440ea7ecc10 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 19:16:24 -0400 Subject: [PATCH 09/18] fix(plugin,maintenance): pin archive checksum and guard cron flock - archive_snapshot_test: verify against the real expected SHA-256 of the original archive bytes instead of comparing the sum with itself. - entitlement_timer: guard cron self-removal with 'command -v flock' so hosts without flock still drop the stale line instead of failing silently and running forever. - Update golden fixture and flock test stubs to match the guarded shell shape. --- internal/maintenance/entitlement_timer.go | 13 +++++++------ internal/maintenance/entitlement_timer_test.go | 4 ++-- .../maintenance/testdata/entitlement_timer/crontab | 2 +- internal/plugin/archive_snapshot_test.go | 3 ++- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/maintenance/entitlement_timer.go b/internal/maintenance/entitlement_timer.go index ec220162f..49cf975ac 100644 --- a/internal/maintenance/entitlement_timer.go +++ b/internal/maintenance/entitlement_timer.go @@ -161,12 +161,13 @@ func renderCron(bin, dir, hash string) (string, error) { if err := checkRenderable(bin, dir); err != nil { return "", err } - minute, _ := strconv.ParseUint(hash[:2], 16, 8) - tag := entCronTag + hash - drop := fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && (flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"; }`, shQuote(tag)) - line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", - minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) - return cronEscape(line), nil +minute, _ := strconv.ParseUint(hash[:2], 16, 8) +tag := entCronTag + hash + // Guard flock: if unavailable, fall back to unlocked removal so the job does not run forever. + drop := fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | { if command -v flock >/dev/null 2>&1; then flock -x 9 crontab - 9>"$HOME/.nself/entitlement-cron.lock"; else crontab -; fi; }; }`, shQuote(tag)) +line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", +minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) +return cronEscape(line), nil } // launchAgentLabel is the launchd label for a project. diff --git a/internal/maintenance/entitlement_timer_test.go b/internal/maintenance/entitlement_timer_test.go index 7f9bd6fae..7052dac4b 100644 --- a/internal/maintenance/entitlement_timer_test.go +++ b/internal/maintenance/entitlement_timer_test.go @@ -528,7 +528,7 @@ func TestCronSelfDeleteCreatesLockDir(t *testing.T) { run := exec.Command("sh", "-c", fields[5]) // Do NOT create ~/.nself; the fix must mkdir -p it before the redirect. if runtime.GOOS == "darwin" { - if err := os.WriteFile(filepath.Join(stubs, "flock"), []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil { + if err := os.WriteFile(filepath.Join(stubs, "flock"), []byte("#!/bin/sh\nshift 2; exec \"$@\"\n"), 0755); err != nil { t.Fatal(err) } } @@ -579,7 +579,7 @@ func TestCronSelfDeleteKeepsForeignLines(t *testing.T) { } // This shell shape is also tested on macOS, where flock is not shipped. if runtime.GOOS == "darwin" { - if err := os.WriteFile(filepath.Join(stubs, "flock"), []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil { + if err := os.WriteFile(filepath.Join(stubs, "flock"), []byte("#!/bin/sh\nshift 2; exec \"$@\"\n"), 0755); err != nil { t.Fatal(err) } } diff --git a/internal/maintenance/testdata/entitlement_timer/crontab b/internal/maintenance/testdata/entitlement_timer/crontab index 5c9251126..c6b6f99c4 100644 --- a/internal/maintenance/testdata/entitlement_timer/crontab +++ b/internal/maintenance/testdata/entitlement_timer/crontab @@ -1 +1 @@ -38 */6 * * * if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else { mkdir -p "$HOME/.nself" && (flock -x 9 && t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | crontab -) 9>"$HOME/.nself/entitlement-cron.lock"; }; fi # nself-entitlement:dae668e4084f07b6 +38 */6 * * * if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else { mkdir -p "$HOME/.nself" && t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | { if command -v flock >/dev/null 2>&1; then flock -x 9 crontab - 9>"$HOME/.nself/entitlement-cron.lock"; else crontab -; fi; }; }; fi # nself-entitlement:dae668e4084f07b6 diff --git a/internal/plugin/archive_snapshot_test.go b/internal/plugin/archive_snapshot_test.go index b2d59b3f2..312d0d4d7 100644 --- a/internal/plugin/archive_snapshot_test.go +++ b/internal/plugin/archive_snapshot_test.go @@ -38,7 +38,8 @@ func TestArchiveSnapshotSurvivesPathReplacement(t *testing.T) { t.Fatal(err) } - if err := verifyChecksumSum(path, sum, sum, "stable"); err != nil { + // Pin the expected hash of the original archive bytes; comparing sum to itself is vacuous. + if err := verifyChecksumSum(path, sum, "2862fa190cd9554fc07cdacaf4ff99cf25c5b72b9dbc5ed5f3d19ea797237c49", "stable"); err != nil { t.Fatal(err) } From 0170db622565e64dcb43d1f245f84276d29f6cc4 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 06:20:22 -0400 Subject: [PATCH 10/18] style(maintenance): gofmt renderCron in entitlement_timer --- internal/maintenance/entitlement_timer.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/maintenance/entitlement_timer.go b/internal/maintenance/entitlement_timer.go index 49cf975ac..7b423f5c4 100644 --- a/internal/maintenance/entitlement_timer.go +++ b/internal/maintenance/entitlement_timer.go @@ -161,13 +161,13 @@ func renderCron(bin, dir, hash string) (string, error) { if err := checkRenderable(bin, dir); err != nil { return "", err } -minute, _ := strconv.ParseUint(hash[:2], 16, 8) -tag := entCronTag + hash + minute, _ := strconv.ParseUint(hash[:2], 16, 8) + tag := entCronTag + hash // Guard flock: if unavailable, fall back to unlocked removal so the job does not run forever. drop := fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | { if command -v flock >/dev/null 2>&1; then flock -x 9 crontab - 9>"$HOME/.nself/entitlement-cron.lock"; else crontab -; fi; }; }`, shQuote(tag)) -line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", -minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) -return cronEscape(line), nil + line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", + minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) + return cronEscape(line), nil } // launchAgentLabel is the launchd label for a project. From 2e428b9c2259c0dceb2faf036223d07523edac14 Mon Sep 17 00:00:00 2001 From: admin Date: Thu, 29 Oct 2026 07:21:51 -0400 Subject: [PATCH 11/18] fix(ci): remove unused isRegularFile from ci_bootstrap --- cmd/commands/ci_bootstrap.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/cmd/commands/ci_bootstrap.go b/cmd/commands/ci_bootstrap.go index f80b2fb51..bd5db2eaa 100644 --- a/cmd/commands/ci_bootstrap.go +++ b/cmd/commands/ci_bootstrap.go @@ -139,14 +139,6 @@ func retireLegacyCICache() { }) } -// isRegularFile reports whether path is itself a regular file. A symlink does -// not count: the installer publishes a copy, so a link there was not put by a -// verified install and falls through to the seam, which replaces it. -func isRegularFile(path string) bool { - info, err := os.Lstat(path) - return err == nil && info.Mode().IsRegular() -} - // needsBuild returns true if the binary is missing or any source file is newer. func needsBuild(binary, pluginDir string) bool { info, err := os.Stat(binary) From e01025924f9c379fab5d74d53c9c0bde30d5fa66 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 10:47:38 -0400 Subject: [PATCH 12/18] fix(maintenance): hold cron lock across the self-removal read-modify-write --- internal/maintenance/entitlement_timer.go | 5 +- .../maintenance/entitlement_timer_test.go | 75 +++++++++++++++++++ .../testdata/entitlement_timer/crontab | 2 +- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/internal/maintenance/entitlement_timer.go b/internal/maintenance/entitlement_timer.go index 7b423f5c4..b3b238c1e 100644 --- a/internal/maintenance/entitlement_timer.go +++ b/internal/maintenance/entitlement_timer.go @@ -163,8 +163,9 @@ func renderCron(bin, dir, hash string) (string, error) { } minute, _ := strconv.ParseUint(hash[:2], 16, 8) tag := entCronTag + hash - // Guard flock: if unavailable, fall back to unlocked removal so the job does not run forever. - drop := fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | { if command -v flock >/dev/null 2>&1; then flock -x 9 crontab - 9>"$HOME/.nself/entitlement-cron.lock"; else crontab -; fi; }; }`, shQuote(tag)) + // The lock (fd 9) spans the whole read-modify-write, so a concurrent install is never overwritten + // with a stale read. Without flock the removal runs unlocked so the job does not run forever. + drop := fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && { if command -v flock >/dev/null 2>&1; then flock -x 9; fi; t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -; } 9>"$HOME/.nself/entitlement-cron.lock"; }`, shQuote(tag)) line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) return cronEscape(line), nil diff --git a/internal/maintenance/entitlement_timer_test.go b/internal/maintenance/entitlement_timer_test.go index 7052dac4b..494ccc430 100644 --- a/internal/maintenance/entitlement_timer_test.go +++ b/internal/maintenance/entitlement_timer_test.go @@ -686,3 +686,78 @@ func TestRefreshGuardRetainsNonExecutableBinary(t *testing.T) { t.Fatalf("existing binary treated as gone: %v %s", err, out) } } + +// TestCronSelfDeleteHoldsLockAcrossReadModifyWrite runs the real cron line while an install +// races it. The crontab stub keeps a slow `-l` (it returns the pre-sleep copy), so a +// self-removal that reads before taking the lock writes that stale copy over the install. +func TestCronSelfDeleteHoldsLockAcrossReadModifyWrite(t *testing.T) { + tmp, _ := filepath.EvalSymlinks(t.TempDir()) + store, stubs := filepath.Join(tmp, "store"), filepath.Join(tmp, "stubs") + if err := os.MkdirAll(stubs, 0o755); err != nil { + t.Fatal(err) + } + stub := "#!/bin/sh\nif [ \"$1\" = -l ]; then cat " + store + "; sleep 0.4; else cat > " + store + "; fi\n" + if err := os.WriteFile(filepath.Join(stubs, "crontab"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + // The shell takes flock on fd 9; a real lock is needed even where flock(1) is not shipped. + if _, err := exec.LookPath("flock"); err != nil { + if _, perr := exec.LookPath("perl"); perr != nil { + t.Skip("neither flock nor perl available") + } + perl := "#!/usr/bin/perl\nuse Fcntl qw(:flock);\nopen(my $f, '>&=', 9) or die;\nflock($f, LOCK_EX) or die;\n" + if err := os.WriteFile(filepath.Join(stubs, "flock"), []byte(perl), 0o755); err != nil { + t.Fatal(err) + } + } + env := []string{"HOME=" + tmp, "PATH=" + stubs + ":" + os.Getenv("PATH")} + bin := filepath.Join(tmp, "nself") + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + gone := filepath.Join(tmp, "gone-project") + line, err := renderCron(bin, gone, Hash(gone)) + if err != nil { + t.Fatal(err) + } + foreign := "MAILTO=ops\n0 3 * * * echo keep\n" + if err := os.WriteFile(store, []byte(foreign+line+"\n"), 0o644); err != nil { + t.Fatal(err) + } + h := host{goos: "linux", crontab: true, home: tmp, run: func(stdin, name string, args ...string) (string, error) { + cmd := exec.Command(filepath.Join(stubs, "crontab"), args...) + cmd.Env = env + if stdin != "" { + cmd.Stdin = strings.NewReader(stdin) + } + out, err := cmd.CombinedOutput() + return string(out), err + }} + other := filepath.Join(tmp, "other-project") + fields := strings.SplitN(strings.ReplaceAll(line, `\%`, "%"), " ", 6) + sh := exec.Command("sh", "-c", fields[5]) + sh.Env = env + if err := os.MkdirAll(filepath.Join(tmp, ".nself"), 0o700); err != nil { + t.Fatal(err) + } + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + time.Sleep(150 * time.Millisecond) // after the shell started reading + if _, err := h.installCron(bin, other, Hash(other)); err != nil { + t.Error(err) + } + }() + if out, err := sh.CombinedOutput(); err != nil { + t.Fatalf("cron line failed: %v\n%s", err, out) + } + wg.Wait() + got, _ := os.ReadFile(store) + if !strings.Contains(string(got), entCronTag+Hash(other)) { + t.Fatalf("self-removal overwrote a concurrent install with a stale copy: %q", got) + } + if strings.Contains(string(got), entCronTag+Hash(gone)) || !strings.Contains(string(got), foreign) { + t.Fatalf("self-removal result wrong: %q", got) + } +} diff --git a/internal/maintenance/testdata/entitlement_timer/crontab b/internal/maintenance/testdata/entitlement_timer/crontab index c6b6f99c4..bd2e7a286 100644 --- a/internal/maintenance/testdata/entitlement_timer/crontab +++ b/internal/maintenance/testdata/entitlement_timer/crontab @@ -1 +1 @@ -38 */6 * * * if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else { mkdir -p "$HOME/.nself" && t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | { if command -v flock >/dev/null 2>&1; then flock -x 9 crontab - 9>"$HOME/.nself/entitlement-cron.lock"; else crontab -; fi; }; }; fi # nself-entitlement:dae668e4084f07b6 +38 */6 * * * if test -e '/usr/local/bin/nself' && test -d '/srv/app'; then env NSELF_V15=1 '/usr/local/bin/nself' license refresh --project '/srv/app' --quiet >/dev/null; else { mkdir -p "$HOME/.nself" && { if command -v flock >/dev/null 2>&1; then flock -x 9; fi; t=$(crontab -l 2>/dev/null) && printf '\%s\n' "$t" | grep -vF '# nself-entitlement:dae668e4084f07b6' | crontab -; } 9>"$HOME/.nself/entitlement-cron.lock"; }; fi # nself-entitlement:dae668e4084f07b6 From c294699f2b0fb2a4c294c74bb1c87d74ae75e7e4 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 12:03:13 -0400 Subject: [PATCH 13/18] fix(maintenance): keep entitlement_timer under the size budget and correct wiki Move the cron self-removal shell into entitlement_cron_drop.go so the file is 298 lines. Update Plugin-Entitlement (test -e guard, shared flock) and Plugin-Security (nself-ci lookup order: dev checkout, then verified install). --- .github/wiki/Plugin-Entitlement.md | 2 +- .github/wiki/Plugin-Security.md | 7 +++---- internal/maintenance/entitlement_cron_drop.go | 10 ++++++++++ internal/maintenance/entitlement_timer.go | 5 +---- 4 files changed, 15 insertions(+), 9 deletions(-) create mode 100644 internal/maintenance/entitlement_cron_drop.go diff --git a/.github/wiki/Plugin-Entitlement.md b/.github/wiki/Plugin-Entitlement.md index a3e35c162..ec510e39f 100644 --- a/.github/wiki/Plugin-Entitlement.md +++ b/.github/wiki/Plugin-Entitlement.md @@ -67,4 +67,4 @@ The first two are checked by a shell guard in the unit, crontab line or LaunchAg **Diagnostics.** E132 (doctor, exit class 1): a licensed v1.5 stack has no active timer. Reasons: `not-installed`, `unsafe-path`, `no-backend`. The hidden `nself license refresh --install-timer-dry-run` prints the units, crontab line and plist that would be installed. -**Limits.** The timer runs with an empty environment as the project owner, so it reads the licence key from the CLI's own store (`nself license set`), never from `NSELF_LICENSE_KEY_*` variables in a shell, and it looks for plugins in the owner's `~/.nself/plugins`. Build as the project owner, not as root on someone else's project: the root build would see root's plugins while the unit (running as the owner) sees none, and removes itself. The guard uses `test -x` on the binary, so a binary that is only briefly not executable during an upgrade also counts as gone. Two builds of different projects at the same instant can race on the user's crontab (the last writer wins); the next build repairs it. The in-place file rewrite is the one build already uses, and the guard keeps its last verified decision through a torn read. Root on a host without systemd gets no timer (`no-backend`): a root crontab would run a binary the project owner can replace. A cron line over about 900 bytes is refused as unsafe-path because cron truncates long lines. If the cache file cannot be written, the scheduled refresh fails with `cache-write-failed` and the licence library may print its own cache warning first. +**Limits.** The timer runs with an empty environment as the project owner, so it reads the licence key from the CLI's own store (`nself license set`), never from `NSELF_LICENSE_KEY_*` variables in a shell, and it looks for plugins in the owner's `~/.nself/plugins`. Build as the project owner, not as root on someone else's project: the root build would see root's plugins while the unit (running as the owner) sees none, and removes itself. The guard uses `test -e` on the binary, so a binary that is briefly missing during an upgrade also counts as gone, but one that is only briefly not executable does not. Cron install and removal share one `flock` (`~/.nself/entitlement-cron.lock`), so two builds of different projects at the same instant do not overwrite each other's crontab edit; on a host without `flock` the self-removal runs unlocked. The in-place file rewrite is the one build already uses, and the guard keeps its last verified decision through a torn read. Root on a host without systemd gets no timer (`no-backend`): a root crontab would run a binary the project owner can replace. A cron line over about 900 bytes is refused as unsafe-path because cron truncates long lines. If the cache file cannot be written, the scheduled refresh fails with `cache-write-failed` and the licence library may print its own cache warning first. diff --git a/.github/wiki/Plugin-Security.md b/.github/wiki/Plugin-Security.md index 13503d631..47e7eb137 100644 --- a/.github/wiki/Plugin-Security.md +++ b/.github/wiki/Plugin-Security.md @@ -67,11 +67,10 @@ Two limits to know: the tier of an entry comes from the registry, so while a tie ### ci gate binary (v1.4) -`nself ci` runs its gates through the `nself-ci` binary of the free `ci` plugin. The CLI finds it in this order and stops at the first hit: +`nself ci` runs its gates through the `nself-ci` binary of the free `ci` plugin. The CLI finds it in this order and stops at the first hit. There is no unverified fast path: a binary already sitting in `~/.nself/plugins/bin/` is not trusted on its own, and the install seam re-checks the published binary under its operation lock. -1. The published plugin binary, `~/.nself/plugins/bin/nself-ci` (a regular file). -2. A dev build from an adjacent plugins checkout (`../plugins/free/ci`), as before. -3. The verified install seam: the CLI installs the signed `ci` plugin from the configured registry (`NSELF_PLUGIN_REGISTRY` or the default). It needs a release signature and a checksum, and the platform package it downloads must carry a platform signature for the CLI's own platform, all checked against the keys compiled into the CLI. If the platform package cannot be downloaded, the installer falls back to the signed source package and publishes a binary only if that package contains one (tracked in `quality/debt.yaml`). A release that fails any of these is refused (E107, E121 to E124) and the error names `nself add ci`. +1. A dev build from an adjacent plugins checkout (`../plugins/free/ci`), as before. A development checkout wins over the installed plugin. +2. The verified install seam: the CLI installs the signed `ci` plugin from the configured registry (`NSELF_PLUGIN_REGISTRY` or the default). It needs a release signature and a checksum, and the platform package it downloads must carry a platform signature for the CLI's own platform, all checked against the keys compiled into the CLI. If the platform package cannot be downloaded, the installer falls back to the signed source package and publishes a binary only if that package contains one (tracked in `quality/debt.yaml`). A release that fails any of these is refused (E107, E121 to E124) and the error names `nself add ci`. `PATH` is never searched, and a Go toolchain is never used to fetch the gate: the old `go install ...@latest` fallback was unpinned and unverified. `nself ci serve` resolves the gate the same way. diff --git a/internal/maintenance/entitlement_cron_drop.go b/internal/maintenance/entitlement_cron_drop.go new file mode 100644 index 000000000..efab09ad3 --- /dev/null +++ b/internal/maintenance/entitlement_cron_drop.go @@ -0,0 +1,10 @@ +package maintenance + +import "fmt" + +// cronDropCmd returns the shell that removes the tagged crontab line. The lock (fd 9) spans the +// whole read-modify-write, so a concurrent install is never overwritten with a stale read. Without +// flock the removal runs unlocked so the job does not run forever. +func cronDropCmd(tag string) string { + return fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && { if command -v flock >/dev/null 2>&1; then flock -x 9; fi; t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -; } 9>"$HOME/.nself/entitlement-cron.lock"; }`, shQuote(tag)) +} diff --git a/internal/maintenance/entitlement_timer.go b/internal/maintenance/entitlement_timer.go index b3b238c1e..682c3a9a4 100644 --- a/internal/maintenance/entitlement_timer.go +++ b/internal/maintenance/entitlement_timer.go @@ -163,11 +163,8 @@ func renderCron(bin, dir, hash string) (string, error) { } minute, _ := strconv.ParseUint(hash[:2], 16, 8) tag := entCronTag + hash - // The lock (fd 9) spans the whole read-modify-write, so a concurrent install is never overwritten - // with a stale read. Without flock the removal runs unlocked so the job does not run forever. - drop := fmt.Sprintf(`{ mkdir -p "$HOME/.nself" && { if command -v flock >/dev/null 2>&1; then flock -x 9; fi; t=$(crontab -l 2>/dev/null) && printf '%%s\n' "$t" | grep -vF %s | crontab -; } 9>"$HOME/.nself/entitlement-cron.lock"; }`, shQuote(tag)) line := fmt.Sprintf("%d */6 * * * if test -e %s && test -d %s; then %s >/dev/null; else %s; fi %s", - minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), drop, tag) + minute%60, shQuote(bin), shQuote(dir), refreshCmd(bin, dir), cronDropCmd(tag), tag) return cronEscape(line), nil } From cea3d28f1bdda2b44d0ea36a5f78dd4ffebd2fc7 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 12:23:06 -0400 Subject: [PATCH 14/18] test(plugin): derive archive snapshot hash at run time so it holds across Go versions --- internal/plugin/archive_snapshot_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/plugin/archive_snapshot_test.go b/internal/plugin/archive_snapshot_test.go index 312d0d4d7..9c272ca39 100644 --- a/internal/plugin/archive_snapshot_test.go +++ b/internal/plugin/archive_snapshot_test.go @@ -4,6 +4,8 @@ import ( "archive/tar" "bytes" "compress/gzip" + "crypto/sha256" + "encoding/hex" "os" "path/filepath" "testing" @@ -22,6 +24,10 @@ func TestArchiveSnapshotSurvivesPathReplacement(t *testing.T) { _, _ = tw.Write([]byte("trusted")) _ = tw.Close() _ = gz.Close() + // The gzip stream differs between Go releases, so derive the expected hash + // from the bytes written to disk rather than from a literal. + original := sha256.Sum256(archive.Bytes()) + want := hex.EncodeToString(original[:]) path := filepath.Join(t.TempDir(), "archive.tgz") if err := os.WriteFile(path, archive.Bytes(), 0600); err != nil { t.Fatal(err) @@ -38,8 +44,9 @@ func TestArchiveSnapshotSurvivesPathReplacement(t *testing.T) { t.Fatal(err) } - // Pin the expected hash of the original archive bytes; comparing sum to itself is vacuous. - if err := verifyChecksumSum(path, sum, "2862fa190cd9554fc07cdacaf4ff99cf25c5b72b9dbc5ed5f3d19ea797237c49", "stable"); err != nil { + // want is hashed from the original archive bytes, so this is not a + // comparison of sum with itself. + if err := verifyChecksumSum(path, sum, want, "stable"); err != nil { t.Fatal(err) } From 9ca1c39cd261d6bd6968abc95c4c2972c489410e Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 13:27:59 -0400 Subject: [PATCH 15/18] fix(build): keep internal/build under its package-size ceiling Restore the internal/plugin ceiling to main's value (the package no longer needs the raise) and tighten the installCert comment so the D-0930 early return fits inside the internal/build ceiling instead of raising it. --- internal/build/shared_proxy_certs.go | 7 ++----- internal/repoqa/testdata/package-size-allowlist.txt | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/build/shared_proxy_certs.go b/internal/build/shared_proxy_certs.go index 0037cdddd..7df01f31b 100644 --- a/internal/build/shared_proxy_certs.go +++ b/internal/build/shared_proxy_certs.go @@ -203,12 +203,9 @@ func (st *buildState) installCert(sp *sharedProxy, dir string, replace bool) err } modes := map[string]os.FileMode{"fullchain.pem": 0o644, "privkey.pem": 0o600} marker := []byte(sp.project + "\n") - // Certificate bytes are a runtime effect (generated during the apply), so a - // plan-bound apply (expectSink) installs them on disk like a plain write when the - // plan does not list them; plan mode routes the pair through its recording sink. + // Cert bytes are a runtime effect: plan mode records the pair in its sink; a plan-bound + // apply (expectSink) stages a pair the plan lists and installs any other pair directly. _, disk := st.sink.(*diskSink) - // A pair the plan already lists (it existed at plan time) goes through the sink, so - // the apply stages and verifies it; only a pair the plan could not list is installed directly. held := false if es, ok := st.sink.(*expectSink); ok { _, planned := es.exp.Files[es.keyr.key(filepath.Join(target, sharedproxy.CertMarker))] diff --git a/internal/repoqa/testdata/package-size-allowlist.txt b/internal/repoqa/testdata/package-size-allowlist.txt index 11d8c3250..b5c9cf6d1 100644 --- a/internal/repoqa/testdata/package-size-allowlist.txt +++ b/internal/repoqa/testdata/package-size-allowlist.txt @@ -10,4 +10,4 @@ internal/config 7498 internal/database 10048 internal/deploy 6667 internal/doctor 9414 -internal/plugin 10092 +internal/plugin 10068 From d13959e5a30098ecd3d4ec126e410792ff7cbe7b Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 13:28:00 -0400 Subject: [PATCH 16/18] chore(ci): regenerate facts.json after rebase onto main --- .github/facts.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/facts.json b/.github/facts.json index f768e823a..54f21a23e 100644 --- a/.github/facts.json +++ b/.github/facts.json @@ -9,7 +9,7 @@ }, "schema_version": "1", "sources": { - "openapi_sha256": "c78ebcd13343a9c05be443c7dbde6226cc8be9019a5384ad969b1522c62d945a", + "openapi_sha256": "46c471ab22d3fcb47cb4232fba5cf7762c7b746b153faaf25e8b6960e31c039c", "registry_sha256": "37fe98fe4dfa26f0c8af15f5cc50e9a5b81f499976850ae2b6066c536e8fe6c9" }, "verbs": [ From 466f27cc6282a3d082248b4783c7e991e7e284d7 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 13:34:59 -0400 Subject: [PATCH 17/18] test(maintenance): take a real fd lock in the cron self-removal test off Linux Homebrew's flock(1) on macOS exits 0 without locking an inherited fd, so the test raced and saw the stale crontab copy. Use the perl flock(2) stub everywhere except Linux, where util-linux flock is trusted. --- internal/maintenance/entitlement_timer_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/maintenance/entitlement_timer_test.go b/internal/maintenance/entitlement_timer_test.go index 494ccc430..d2d13a270 100644 --- a/internal/maintenance/entitlement_timer_test.go +++ b/internal/maintenance/entitlement_timer_test.go @@ -700,8 +700,9 @@ func TestCronSelfDeleteHoldsLockAcrossReadModifyWrite(t *testing.T) { if err := os.WriteFile(filepath.Join(stubs, "crontab"), []byte(stub), 0o755); err != nil { t.Fatal(err) } - // The shell takes flock on fd 9; a real lock is needed even where flock(1) is not shipped. - if _, err := exec.LookPath("flock"); err != nil { + // The shell takes flock on fd 9. Only util-linux flock(1) locks an inherited fd (macOS ports + // such as Homebrew's do not), so off Linux, or without it, a perl stub takes flock(2) on fd 9. + if _, err := exec.LookPath("flock"); err != nil || runtime.GOOS != "linux" { if _, perr := exec.LookPath("perl"); perr != nil { t.Skip("neither flock nor perl available") } From b3481163eb4d2663d1d5c8e20fa69d2401a68c04 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 18:52:52 -0400 Subject: [PATCH 18/18] chore(ci): regenerate facts.json after rebase onto main --- .github/facts.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/facts.json b/.github/facts.json index 54f21a23e..ad07f7072 100644 --- a/.github/facts.json +++ b/.github/facts.json @@ -9,7 +9,7 @@ }, "schema_version": "1", "sources": { - "openapi_sha256": "46c471ab22d3fcb47cb4232fba5cf7762c7b746b153faaf25e8b6960e31c039c", + "openapi_sha256": "1f47f7be7d98810ce4eca08fb61bd930cc662290040b162bfc96fe6e602a6059", "registry_sha256": "37fe98fe4dfa26f0c8af15f5cc50e9a5b81f499976850ae2b6066c536e8fe6c9" }, "verbs": [