Files
zero/internal/cli/hook_dispatch.go
beardthelionandKevin Codex a880ce80a6 feat: gate project-scoped hooks, plugins, and MCP servers behind workspace trust (#529)
* feat(workspacetrust): add exact-match trust store for project config gating

* feat(hooks): add ExcludeProject option to LoadConfig

* feat(plugins): add ExcludeProject option to Load

* feat(cli): gate project hooks and plugins behind workspace trust

Trust check lives inside newHookDispatcherWithExtra and activatePlugins
(fail-closed by construction); trustRoot is the original launch dir so a
--worktree run inherits the source repo's trust. Callers OR the skip
reports and emit one combined stderr notice.

* feat(cli): add zero trust command (trust/list/remove)

* fix(review): key spec-draft trust on the original launch dir; cover trust CLI edge branches

Spec-draft runs (zero exec --use-spec --worktree) passed the reassigned
worktree path as trustRoot, dropping a trusted repo's project hooks
(fail-closed, but breaks worktree-inheritance). Thread the captured
pre-worktree trustRoot into execSpecDraftRun. Add tests for the trust
'remove' too-many-args and --help branches, and a resolveTrust comment
noting the per-chokepoint check must not be hoisted to callers.

* test(cli): end-to-end trust-gate coverage through agent.Run and exec --worktree

Drives the real agent loop with a fake tool-calling provider so a project
beforeTool hook actually fires (or is gated away) through production code:
gate blocks a tool call via agent.Run, and --worktree / --use-spec --worktree
key trust on the original launch dir. All three are load-bearing (RED when the
gate or the trustRoot threading regresses).

* style(cli): gofmt struct alignment in exec_spec.go

* fix(workspacetrust): compare stored trust entries literally, no symlink re-resolution

External adversarial review (codex) found a false positive: IsTrusted
re-normalized stored entries on every check (Abs+EvalSymlinks), so a trusted
path later replaced by a symlink to attacker content re-resolved to the new
target and matched. Stored entries are already canonical (normalized once at
Trust time); compare them as literals and only normalize the incoming query.
Add a regression test for the symlink-retarget case.

* feat(mcp): gate project-scoped MCP servers behind workspace trust

Closes the P0 an external adversarial review (codex) found: project
./.zero/config.json stdio MCP servers spawned before the hook/plugin gate,
an ungated arbitrary-execution path that could also self-trust the workspace.
Add ExcludeProject to config.ResolveMCP (skips the project layer) and thread a
fail-closed trust check into every MCP spawn site (exec, TUI setup+refresh,
mcp tools list, mcp check); reporting sites stay ungated. Verified the sole
server-command exec (mcp/client.go) is reachable only through the now-gated
RegisterTools paths.

* test(cli): cover default-mode beforeTool firing through the trust gate

Assert a project beforeTool hook fires in the default auto and ask permission
modes (not only under unsafe), and is blocked when the workspace is untrusted,
so the gate is verified in the mode a normal run actually uses.

* docs(cli): note MCP servers in zero trust help text and doc comment

The gate covers project hooks, plugins, and MCP servers, but the help string and
runTrust doc comment listed only hooks and plugins.

* test(cli): make workspace-trust tests portable on macOS and Windows

setTrustConfigRoot set HOME on darwin, but config.UserConfigDir is XDG-first
there (=$HOME/.config), so tests wrote trust.json and user hooks.json where the
code never read them; set XDG_CONFIG_HOME on every non-Windows platform instead.

writeMarkerHook shells out to /bin/sh, absent on Windows, so its trusted-case
marker never appeared; skip it there like writeMarkerHookScript already does.

TestRunTrustAddThenList compared `trust list` output (canonicalized via
EvalSymlinks) against the raw cwd, which diverges on the Windows runner's 8.3
short names (RUNNER~1 -> runneradmin); compare the resolved path, and add
TestRunTrustListShowsCanonicalPath to exercise that via a symlink on any platform.

* feat(cli): surface the project MCP skip in the workspace-trust notice

Project MCP servers were dropped silently in an untrusted workspace: the trust
notice covered hooks and plugins but not MCP, and app.go discarded the
resolveTrust error. registerMCPToolsForWorkspace now returns a trustSkip, gated
on a new projectMCPConfigExists check plus the previously-dropped store-read
error, and emitTrustNotice is variadic so the exec, spec-draft, interactive, and
`mcp tools list` paths all fold in the MCP skip.

The skip drives the notice only; the gate stays on resolveTrust/excludeProject,
so a trusted workspace is unaffected and an untrusted one still fails closed.

* fix(cli): print zero trust --help to stdout with a success exit

zero trust -h/--help/help routed usage to stderr with the usage exit code, unlike
every other subcommand (mcp, sandbox, skills, cron, ...) which prints help to
stdout and returns success. Align it; the unknown-subcommand path still writes to
stderr with the usage exit code.

* fix(cli): gate oauth MCP login and surface mcp-check trust notice

Two project-scoped MCP activation sites were left outside the workspace
trust gate.

zero mcp oauth login loaded a project-defined OAuth server from an
untrusted workspace and ran the full discovery/registration/token-exchange
flow against its configured URL. Gate resolveOAuthServer with resolveTrust
so an untrusted clone's ./.zero/config.json OAuth server is dropped before
login, and emit the same one-line trust notice the other spawn sites use.

zero mcp check already gated the project layer but discarded the trust-check
result and never emitted the notice, so a gated project server read as a
bare "not configured". Emit the notice on the not-found branch.

The three remaining ungated resolveMCPConfig(cwd, false) sites (backends
doctor/snapshot and the extensions listing) are enumeration-only and stay
ungated.

* test(cli): adapt MCP startup-skip tests to two-arg resolveMCPConfig

---------

Co-authored-by: Kevin Codex <[email protected]>
2026-07-09 10:51:09 +08:00

152 lines
6.4 KiB
Go

package cli
import (
"fmt"
"io"
"os"
"path/filepath"
"github.com/Gitlawb/zero/internal/hooks"
"github.com/Gitlawb/zero/internal/workspacetrust"
)
// trustSkip reports whether a runtime chokepoint dropped the project layer because
// the workspace was not trusted, so the caller can emit a single combined notice.
// Both chokepoints (hooks and plugins) return one, and the caller ORs them.
type trustSkip struct {
// excludedProjectConfig is true when the project layer was excluded AND a
// project config for this surface actually exists on disk (so a notice is
// worth showing). It stays false in a trusted workspace, or when the
// workspace has no project config to skip.
excludedProjectConfig bool
// trustCheckErrored is true when the exclusion was caused by a trust-store
// read error rather than a clean untrusted verdict, so the caller can name
// the error in its notice.
trustCheckErrored bool
}
// resolveTrust computes the fail-closed trust verdict for a chokepoint. It treats
// any error OR an empty trustRoot as untrusted, so a forgotten or future call site
// cannot fail open. It returns whether the project layer should be excluded and
// whether the decision was driven by a store-read error.
//
// Each chokepoint (hooks and plugins) calls this itself rather than the caller
// resolving trust once and passing a bool down. That is deliberate: keeping the
// check inside the chokepoint is what makes the gate fail-closed by construction.
// Do NOT hoist it to the callers behind an excludeProject bool whose zero value
// includes the project layer, that reintroduces a fail-open default. The extra
// store read per session is negligible.
func resolveTrust(trustRoot string) (excludeProject bool, trustCheckErrored bool) {
if trustRoot == "" {
return true, false
}
trusted, err := workspacetrust.IsTrusted(trustRoot)
if err != nil {
return true, true
}
return !trusted, false
}
// newHookDispatcher builds the per-session hooks dispatcher for a workspace,
// merging user + project hooks.json and wiring the audit store. It fails OPEN:
// any load or setup error yields a nil dispatcher, which Dispatch treats as a
// no-op, so a malformed hooks config can never wedge tool execution. With no
// hooks configured the dispatcher selects nothing and runs no commands, so the
// hot path stays free of overhead until a user opts in via hooks.json.
//
// trustRoot is the original launch directory (resolved before any --worktree
// reassignment); the project layer loads only when that root is trusted.
func newHookDispatcher(workspaceRoot string, trustRoot string) (*hooks.Dispatcher, trustSkip) {
return newHookDispatcherWithExtra(workspaceRoot, nil, trustRoot)
}
// newHookDispatcherWithExtra builds the dispatcher like newHookDispatcher but also
// folds plugin-activated hook definitions into the active hook set, so a plugin's
// declared hooks run alongside the user/project hooks.json hooks. Plugin hooks are
// appended after the configured hooks; their ids are plugin-namespaced (plugin
// id + hook name) so they never collide with hooks.json ids. A nil/empty extra
// slice is byte-equivalent to newHookDispatcher.
//
// The trust check lives here, inside the chokepoint, so no caller can bypass it:
// the project layer is dropped (ExcludeProject) whenever trustRoot is empty, the
// trust store cannot be read, or the workspace is not trusted (fail-closed). The
// returned trustSkip lets the caller emit one combined notice; the notice itself
// is NOT emitted here.
func newHookDispatcherWithExtra(workspaceRoot string, extra []hooks.Definition, trustRoot string) (*hooks.Dispatcher, trustSkip) {
excludeProject, trustCheckErrored := resolveTrust(trustRoot)
skip := trustSkip{
excludedProjectConfig: excludeProject && projectHooksFileExists(workspaceRoot),
trustCheckErrored: trustCheckErrored,
}
loaded, err := hooks.LoadConfig(hooks.LoadOptions{Cwd: workspaceRoot, ExcludeProject: excludeProject})
if err != nil {
return nil, skip
}
var audit *hooks.AuditStore
if store, err := hooks.NewAuditStore(hooks.AuditStoreOptions{}); err == nil {
audit = store
}
config := loaded.Config
if len(extra) > 0 {
// Plugin hooks only run when hooks are enabled overall; an explicit
// `enabled:false` in hooks.json still disables the whole hook surface.
merged := append([]hooks.Definition{}, config.Hooks...)
existing := make(map[string]bool, len(merged))
for _, hook := range merged {
existing[hook.ID] = true
}
for _, hook := range extra {
// A hooks.json hook with the same (namespaced) id wins, so an operator can
// still disable a plugin hook by id without the plugin re-enabling it.
if existing[hook.ID] {
continue
}
merged = append(merged, hook)
}
config.Hooks = merged
}
return hooks.NewDispatcher(hooks.DispatcherOptions{
Config: config,
Audit: audit,
Cwd: workspaceRoot,
}), skip
}
// projectHooksFileExists reports whether a ./.zero/hooks.json is present under
// workspaceRoot, so the caller only notices about config it actually skipped.
func projectHooksFileExists(workspaceRoot string) bool {
if workspaceRoot == "" {
return false
}
info, err := os.Stat(filepath.Join(workspaceRoot, ".zero", "hooks.json"))
return err == nil && !info.IsDir()
}
// emitTrustNotice writes at most one stderr line summarizing that project-scoped
// hooks, plugins, and/or MCP servers were skipped in an untrusted workspace. It
// takes the skip report from each trust-gated surface (hooks, plugins, MCP) and ORs
// them, so a workspace that only drops one surface still gets the notice. It is
// computed once per session by the caller (each session-setup site runs once), so it
// is naturally once-per-process. When any surface's skip was a trust-store read
// error, the notice names that so a transient config-dir problem is diagnosable.
func emitTrustNotice(stderr io.Writer, skips ...trustSkip) {
if stderr == nil {
return
}
excluded := false
storeErrored := false
for _, skip := range skips {
excluded = excluded || skip.excludedProjectConfig
storeErrored = storeErrored || skip.trustCheckErrored
}
if !excluded {
return
}
if storeErrored {
_, _ = fmt.Fprintln(stderr, "zero: the workspace-trust store could not be read; ignoring project hooks/plugins/MCP servers (fail-closed). Run 'zero trust' to enable.")
return
}
_, _ = fmt.Fprintln(stderr, "zero: ignoring project hooks/plugins/MCP servers in an untrusted workspace. Run 'zero trust' to enable.")
}