Skip to content

Wire applyInstallGate (and runAllPluginCards) into production paths #116

Description

@alamb-hex

Summary

The security-refactor PR shipped two pieces of plugin scaffolding that have full unit-test coverage but no production caller:

  • src/lib/security/plugins/apply-install-gate.ts — orchestrator that chains wrapInstall left-to-right across all enabled installGate plugins and fans onResult back R→L to surface the first blocker. 4 unit tests cover empty-plugins, chaining, first-block detection, and all-clear paths.
  • src/lib/security/plugins/runner.ts (runAllPluginCards) — parallel runner with isolated per-plugin errors. 5 unit tests including a parallelism timing assertion.

What the production code does today:

  • /api/projects/[id]/update/route.ts asks each enabled installGate plugin's wrapInstall(...) directly and picks the first one whose command[0] differs from packageManager (break; // first enabled plugin wins; chaining is a future story). Threads the resulting binary as installBinOverride into installPackages. Single-plugin only; no chaining; no onResult callback at all.
  • /security/page.tsx fetches /api/security/plugins then per-plugin /api/security/plugins/[id]/status?projectId=… via raw Promise.all. Doesn't call runAllPluginCards; builds PluginCardEntry[] inline from the two responses.

Both decisions are reasonable for the current state (one installGate plugin, one consumer of the cards row), but they leave the orchestrator + runner orphaned in production and create a fork-in-the-road problem when a second installGate plugin (Socket Firewall, future tools) lands.

Why this matters

  • Footgun risk: a future contributor adds Socket Firewall → wires it into /update by extending the ad-hoc loop with its own block-detection logic, missing the orchestrator that already handles chaining + onResult fan-out.
  • Dead-code drift: a future cleanup pass may simply delete apply-install-gate.ts thinking it's unused, breaking the documented architecture (spec §6.1, §7.2).
  • onResult is permanently lost: the current /update wiring lets non-zero exits bubble through the existing error path. That works for Safe Chain (its BLOCKED message ends up in stderr → install error → response). But the rich { blocked, message, advisoryRefs } shape that parseSafeChainResult produces is computed in tests only, never in production. Audit log entry only gets { plugin, binOverride }, not the advisory refs.

Options

  1. Wire it now — replace the ad-hoc loop in /update/route.ts with applyInstallGate({ project, command, env, plugins }). Use gate.command[0] for the binary override pass-through to installPackages. After the install, call gate.processResult({ code, stdout, stderr }) to get the rich block summary; thread firstBlocker.advisoryRefs into the audit log meta. Same on /security/page.tsx for the cards row.
  2. Add a TODO comment at the ad-hoc loop pointing at the orchestrator so future readers don't think it's dead code. Defer the actual wiring until a 2nd installGate plugin lands.

Recommend #1 — the orchestrator's per-plugin onResult is the only place we'd ever surface a structured "Safe Chain blocked because of advisory X" UI; the current path drops that information.

Tracking

  • PR that introduced the scaffolding: feat+security-plugins-and-restyle (commits a47871b for orchestrator, d010260 for runAllPluginCards)
  • Related: spec doc docs/superpowers/specs/2026-05-26-security-refactor-design.md §6.1, §7.2 (gitignored, local-only)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions