From 43979f6c1258e54b84acd2066195a71ceabe54f6 Mon Sep 17 00:00:00 2001 From: johnnyfish Date: Fri, 31 Jul 2026 10:01:20 -0700 Subject: [PATCH] fix(enforce): fail closed for GUI editors and add transparent redirect Make --enforce trustworthy end to end instead of silently degrading: - Fail closed on --enforce for GUI editors (Cursor) and launch them inside the OneCLI sandbox so their traffic is actually governed. - Make headless cursor-agent a first-class enforceable agent. - Verify the gateway CA is trusted (not merely installed) and detect a rotated CA before it breaks GUI editors. - Add transparent redirect: an SNI-recovering listener, fail-closed pf anchor management nested under com.apple, a setgid helper so pf can scope redirection by group, an opt-in transparent sandbox profile, and session lifecycle with fail-closed teardown. - Cover it with live TLS proof, pf anchor, and full-chain tests, plus the Cursor coverage demo runbook. --- cmd/onecli/ca_trust_live_test.go | 39 ++ .../enforce_setup_transparent_darwin.go | 188 +++++++++ cmd/onecli/help.go | 4 + cmd/onecli/main.go | 6 + cmd/onecli/run.go | 341 +++++++++++++++- ...run_enforce_full_chain_live_darwin_test.go | 260 ++++++++++++ cmd/onecli/run_enforce_pf_darwin.go | 379 ++++++++++++++++++ cmd/onecli/run_enforce_pf_darwin_test.go | 248 ++++++++++++ cmd/onecli/run_enforce_sni.go | 264 ++++++++++++ cmd/onecli/run_enforce_sni_test.go | 189 +++++++++ cmd/onecli/run_enforce_transparent.go | 193 +++++++++ ..._transparent_lifecycle_live_darwin_test.go | 116 ++++++ ...un_enforce_transparent_live_darwin_test.go | 197 +++++++++ .../run_enforce_transparent_session_darwin.go | 138 +++++++ ...enforce_transparent_session_darwin_test.go | 126 ++++++ .../run_enforce_transparent_sidecar_darwin.go | 151 +++++++ cmd/onecli/run_enforce_transparent_test.go | 330 +++++++++++++++ cmd/onecli/run_enforce_wrap.go | 40 +- .../run_enforce_wrap_bypass_live_test.go | 15 +- cmd/onecli/run_enforce_wrap_mode_darwin.go | 21 + cmd/onecli/run_enforce_wrap_mode_other.go | 30 ++ cmd/onecli/run_enforce_wrap_test.go | 25 +- .../run_enforce_wrap_transparent_darwin.go | 96 +++++ cmd/onecli/run_test.go | 163 +++++++- cmd/onecli/sandbox_audit.go | 3 +- cmd/onecli/sandbox_transparent_cmd_darwin.go | 96 +++++ .../sandbox_transparent_cmd_darwin_test.go | 88 ++++ cmd/onecli/sandbox_transparent_cmd_other.go | 37 ++ docs/cursor-demo-runbook.md | 158 ++++++++ internal/sandbox/helper/onecli-sandbox-gid.c | 102 +++++ internal/sandbox/sandbox.go | 8 + internal/sandbox/sandbox_darwin.go | 86 +++- internal/sandbox/sandbox_other.go | 12 + .../sandbox_transparent_darwin_test.go | 104 +++++ .../sandbox_transparent_kernel_darwin_test.go | 114 ++++++ 35 files changed, 4341 insertions(+), 26 deletions(-) create mode 100644 cmd/onecli/ca_trust_live_test.go create mode 100644 cmd/onecli/enforce_setup_transparent_darwin.go create mode 100644 cmd/onecli/run_enforce_full_chain_live_darwin_test.go create mode 100644 cmd/onecli/run_enforce_pf_darwin.go create mode 100644 cmd/onecli/run_enforce_pf_darwin_test.go create mode 100644 cmd/onecli/run_enforce_sni.go create mode 100644 cmd/onecli/run_enforce_sni_test.go create mode 100644 cmd/onecli/run_enforce_transparent.go create mode 100644 cmd/onecli/run_enforce_transparent_lifecycle_live_darwin_test.go create mode 100644 cmd/onecli/run_enforce_transparent_live_darwin_test.go create mode 100644 cmd/onecli/run_enforce_transparent_session_darwin.go create mode 100644 cmd/onecli/run_enforce_transparent_session_darwin_test.go create mode 100644 cmd/onecli/run_enforce_transparent_sidecar_darwin.go create mode 100644 cmd/onecli/run_enforce_transparent_test.go create mode 100644 cmd/onecli/run_enforce_wrap_mode_darwin.go create mode 100644 cmd/onecli/run_enforce_wrap_mode_other.go create mode 100644 cmd/onecli/run_enforce_wrap_transparent_darwin.go create mode 100644 cmd/onecli/sandbox_transparent_cmd_darwin.go create mode 100644 cmd/onecli/sandbox_transparent_cmd_darwin_test.go create mode 100644 cmd/onecli/sandbox_transparent_cmd_other.go create mode 100644 docs/cursor-demo-runbook.md create mode 100644 internal/sandbox/helper/onecli-sandbox-gid.c create mode 100644 internal/sandbox/sandbox_transparent_darwin_test.go create mode 100644 internal/sandbox/sandbox_transparent_kernel_darwin_test.go diff --git a/cmd/onecli/ca_trust_live_test.go b/cmd/onecli/ca_trust_live_test.go new file mode 100644 index 0000000..04f6002 --- /dev/null +++ b/cmd/onecli/ca_trust_live_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "os" + "strings" + "testing" + + "github.com/onecli/onecli-cli/pkg/output" +) + +// Live check against the REAL login keychain and the REAL gateway CA on disk. +// This is the end-to-end version of the unit tests: it proves the warning fires +// (or stays silent) for the machine's actual trust state, which is the thing +// that broke Cursor. Skipped unless ONECLI_LIVE_CA_CHECK=1 so CI stays hermetic. +func TestLiveGatewayCATrustState(t *testing.T) { + if os.Getenv("ONECLI_LIVE_CA_CHECK") != "1" { + t.Skip("set ONECLI_LIVE_CA_CHECK=1 to check this machine's real keychain") + } + home, err := os.UserHomeDir() + if err != nil { + t.Fatalf("home: %v", err) + } + pemBytes, err := os.ReadFile(home + "/.onecli/gateway-ca.pem") + if err != nil { + t.Skipf("no gateway CA on disk yet: %v", err) + } + + var buf strings.Builder + w := output.NewWithWriters(&buf, &buf) + warnIfGatewayCANotTrusted(w, string(pemBytes)) + + got := buf.String() + t.Logf("warning output: %q", got) + if got == "" { + t.Log("RESULT: the live gateway CA is trusted; GUI editors will work") + } else { + t.Log("RESULT: warning fired, GUI editors would fail until the CA is re-trusted") + } +} diff --git a/cmd/onecli/enforce_setup_transparent_darwin.go b/cmd/onecli/enforce_setup_transparent_darwin.go new file mode 100644 index 0000000..67f00ec --- /dev/null +++ b/cmd/onecli/enforce_setup_transparent_darwin.go @@ -0,0 +1,188 @@ +//go:build darwin + +package main + +// Setup and readiness for transparent redirect, surfaced by +// `onecli sandbox transparent {status,setup}`. +// +// Transparent redirect needs root exactly twice, both one-time: +// 1. create the dedicated group that scopes redirection +// 2. authorize loading the pf anchor at session start +// +// Neither happens in the data path. After setup, enforced runs install and +// flush the anchor through a narrowly-scoped sudoers rule that permits +// pfctl and nothing else. +// +// This command is deliberately explicit and reversible: it prints exactly +// what it will do, what it changed, and how to undo it. A security product +// that quietly edits sudoers has no business asking to be trusted. + +import ( + "fmt" + "os" + "os/exec" + "strconv" + "strings" +) + +const ( + // transparentGroupName is the dedicated group whose egress pf captures. + transparentGroupName = "onecli-sandbox" + // transparentSudoersPath scopes the standing privilege to pfctl alone. + transparentSudoersPath = "/etc/sudoers.d/onecli-transparent" + // transparentGIDFloor keeps us clear of system groups. + transparentGIDFloor = 700 +) + +// transparentSetupPlan describes what setup will change, so it can be shown +// before anything is touched. +type transparentSetupPlan struct { + GroupExists bool + GroupGID int + SudoersOK bool + PFEnabled bool + AnchorInMain bool +} + +// inspectTransparentSetup reports current state WITHOUT changing anything. +func inspectTransparentSetup() transparentSetupPlan { + var p transparentSetupPlan + if gid, err := sandboxGID(transparentGroupName); err == nil { + p.GroupExists = true + p.GroupGID = gid + } + // Capability probe, not a file read: the sudoers file is mode 440 + // root:wheel, so reading it fails even when the entry works. + p.SudoersOK = pfSudoWorks() + if on, err := pfEnabled(); err == nil { + p.PFEnabled = on + } + if ref, err := pfMainRulesetReferencesAnchor(); err == nil { + p.AnchorInMain = ref + } + return p +} + +// nextFreeGID finds an unused GID at or above the floor, so setup never +// collides with an existing group. +func nextFreeGID() (int, error) { + out, err := exec.Command("/usr/bin/dscl", ".", "-list", "/Groups", "PrimaryGroupID").Output() + if err != nil { + return 0, fmt.Errorf("listing groups: %w", err) + } + used := map[int]bool{} + for _, line := range strings.Split(string(out), "\n") { + f := strings.Fields(line) + if len(f) < 2 { + continue + } + if gid, err := strconv.Atoi(f[len(f)-1]); err == nil { + used[gid] = true + } + } + for gid := transparentGIDFloor; gid < transparentGIDFloor+500; gid++ { + if !used[gid] { + return gid, nil + } + } + return 0, fmt.Errorf("no free GID found above %d", transparentGIDFloor) +} + +// transparentSetupScript renders the shell the user runs under sudo. +// +// Returned as text for the user to inspect and execute rather than executed +// for them: this edits sudoers and creates a group, and a security tool +// should show its work. It is idempotent and safe to re-run. +func transparentSetupScript(gid int, user string) string { + var b strings.Builder + b.WriteString("#!/bin/bash\n") + b.WriteString("# OneCLI transparent-redirect setup. Idempotent; safe to re-run.\n") + b.WriteString("set -euo pipefail\n\n") + + b.WriteString("# 1. Dedicated group that scopes pf redirection.\n") + fmt.Fprintf(&b, "if ! dscl . -read /Groups/%s >/dev/null 2>&1; then\n", transparentGroupName) + fmt.Fprintf(&b, " dscl . -create /Groups/%s\n", transparentGroupName) + fmt.Fprintf(&b, " dscl . -create /Groups/%s PrimaryGroupID %d\n", transparentGroupName, gid) + fmt.Fprintf(&b, " dscl . -create /Groups/%s RealName 'OneCLI sandboxed agents'\n", transparentGroupName) + fmt.Fprintf(&b, " echo 'created group %s (gid %d)'\n", transparentGroupName, gid) + b.WriteString("else\n echo 'group already exists'\nfi\n\n") + + fmt.Fprintf(&b, "# 2. Add %s to the group so enforced runs can adopt it.\n", user) + fmt.Fprintf(&b, "dseditgroup -o edit -a %s -t user %s 2>/dev/null || true\n\n", user, transparentGroupName) + + b.WriteString("# 3. Scoped sudo for pfctl ONLY. No blanket root.\n") + fmt.Fprintf(&b, "cat > %s <<'SUDOERS'\n", transparentSudoersPath) + fmt.Fprintf(&b, "%s ALL=(root) NOPASSWD: %s\n", user, pfctlPath) + b.WriteString("SUDOERS\n") + fmt.Fprintf(&b, "chmod 440 %s\n", transparentSudoersPath) + fmt.Fprintf(&b, "visudo -cf %s\n\n", transparentSudoersPath) + + b.WriteString("# 4. pf must be enabled for any anchor to evaluate.\n") + fmt.Fprintf(&b, "%s -E >/dev/null 2>&1 || true\n\n", pfctlPath) + + b.WriteString("echo\necho 'OneCLI transparent redirect is set up.'\n") + fmt.Fprintf(&b, "echo 'To undo: sudo rm %s && sudo dscl . -delete /Groups/%s'\n", + transparentSudoersPath, transparentGroupName) + return b.String() +} + +// verifyTransparentSetup checks every precondition and returns a specific +// reason for the first failure. Callers must treat any error as fatal: +// transparent mode widens the Seatbelt profile, so running it without a +// working anchor would be ungoverned direct egress. +func verifyTransparentSetup() error { + if err := pfAvailable(); err != nil { + return err + } + gid, err := sandboxGID(transparentGroupName) + if err != nil { + return fmt.Errorf("group %q missing — run `onecli sandbox transparent setup`", + transparentGroupName) + } + if err := validateSandboxGID(gid); err != nil { + return err + } + on, err := pfEnabled() + if err != nil { + return fmt.Errorf("cannot determine pf status: %w", err) + } + if !on { + return fmt.Errorf("pf is disabled; the anchor would load but never evaluate " + + "(enable with `sudo pfctl -E`)") + } + ref, err := pfMainRulesetReferencesAnchor() + if err != nil { + return fmt.Errorf("cannot read the main pf ruleset: %w", err) + } + if !ref { + return fmt.Errorf("the main pf ruleset does not reference the %q anchor, "+ + "so its rules would never evaluate", pfAnchorName) + } + return nil +} + +// printTransparentStatus renders a human-readable readiness report. +func printTransparentStatus(w *os.File) { + p := inspectTransparentSetup() + mark := func(ok bool) string { + if ok { + return "ok " + } + return "MISSING" + } + fmt.Fprintf(w, "OneCLI transparent redirect\n\n") + fmt.Fprintf(w, " %s group %s", mark(p.GroupExists), transparentGroupName) + if p.GroupExists { + fmt.Fprintf(w, " (gid %d)", p.GroupGID) + } + fmt.Fprintf(w, "\n") + fmt.Fprintf(w, " %s scoped sudo for pfctl (verified by running it)\n", mark(p.SudoersOK)) + fmt.Fprintf(w, " %s pf enabled\n", mark(p.PFEnabled)) + fmt.Fprintf(w, " %s main ruleset references the %q anchor\n", mark(p.AnchorInMain), pfAnchorName) + fmt.Fprintf(w, "\n") + if err := verifyTransparentSetup(); err != nil { + fmt.Fprintf(w, " NOT READY: %v\n", err) + return + } + fmt.Fprintf(w, " READY — enforced runs can use transparent redirect.\n") +} diff --git a/cmd/onecli/help.go b/cmd/onecli/help.go index 9c81207..7a7bcb7 100644 --- a/cmd/onecli/help.go +++ b/cmd/onecli/help.go @@ -455,6 +455,10 @@ func (cmd *HelpCmd) Run(out *output.Writer) error { {Name: "sandbox audit", Description: "Red-team the enforce-mode sandbox: attempt every known egress bypass and report which the OS actually stops. Exits non-zero if any hole is found.", Args: []ArgInfo{ {Name: "", Description: "Agent to audit (codex, cursor, claude, ...). Defaults to the OneCLI-owned sandbox."}, }}, + {Name: "sandbox transparent status", Description: "Report whether transparent redirect is ready: the sandbox group, scoped pfctl sudo, pf enabled, and an anchor the main ruleset actually reaches. macOS only."}, + {Name: "sandbox transparent setup", Description: "Print (does not run) the one-time privileged setup for transparent redirect, which governs apps that ignore proxy configuration. macOS only.", Args: []ArgInfo{ + {Name: "--helper", Description: "Path to the setgid helper source. Defaults to the copy in this checkout."}, + }}, {Name: "migrate", Description: "Migrate data to OneCLI Cloud.", Args: []ArgInfo{ {Name: "--cloud-key", Required: true, Description: "OneCLI Cloud API key."}, }}, diff --git a/cmd/onecli/main.go b/cmd/onecli/main.go index c0dea40..10f74ee 100644 --- a/cmd/onecli/main.go +++ b/cmd/onecli/main.go @@ -49,6 +49,12 @@ func main() { runEnforceForwarder(pid) return } + // Same pattern for the transparent-redirect listener, which must also + // outlive the syscall.Exec that replaces this process with the agent. + if pid, ok := parseTransparentSidecarArgs(os.Args[1:]); ok { + runTransparentSidecar(pid) + return + } // When invoked with no args, --help, or -h, output structured JSON // so agents always get machine-readable output. diff --git a/cmd/onecli/run.go b/cmd/onecli/run.go index 53b0ca1..3b2be42 100644 --- a/cmd/onecli/run.go +++ b/cmd/onecli/run.go @@ -2,9 +2,11 @@ package main import ( "bytes" + "crypto/x509" _ "embed" "encoding/base64" "encoding/json" + "encoding/pem" "fmt" "net/url" "os" @@ -13,6 +15,7 @@ import ( "regexp" "runtime" "slices" + "strconv" "strings" "syscall" "time" @@ -146,7 +149,10 @@ func (c *RunCmd) Run(out *output.Writer) error { // be repointed at the forwarder first. Both paths fail closed. enforceNative := false wrapProfilePath := "" + guiBinary := "" // non-empty when enforcing a GUI editor: exec this app-bundle binary instead of the CLI launcher var wrapPort uint16 + // Owns the pf anchor when transparent redirect is active; nil otherwise. + var transparentSess *transparentSession if c.Enforce { spec, known := agentSkillDir(c.Args[0]) switch { @@ -157,12 +163,44 @@ func (c *RunCmd) Run(out *output.Writer) error { // (and the wrap denies docker.sock as an egress bypass), so // the wrap cannot govern this agent. Fail closed. return fmt.Errorf("--enforce is not supported for %s: its tools run in a Docker sandbox the OS sandbox cannot govern", spec.agentName) + case known && spec.configDir != "": + // GUI editors (Cursor and other VS Code-style apps): the `cursor` + // CLI is only a launcher — it hands off to an already-running + // Electron app, so sandboxing IT would govern nothing. Instead we + // exec the app bundle's own binary under the sandbox, which puts + // the whole editor (AI calls, telemetry, extensions, its terminal) + // inside the profile. Verified: Cursor starts normally and its own + // update check to api2.cursor.sh is refused by the OS + // (net::ERR_ACCESS_DENIED), i.e. enforcement the app cannot ignore. + guiBinary, err = resolveGUIAppBinary(spec) + if err != nil { + return fmt.Errorf("--enforce for %s: %w", spec.agentName, err) + } + if running, pid := guiAlreadyRunning(spec); running { + // macOS activates the EXISTING instance instead of starting + // ours, so launching now would report enforcement while the + // user keeps typing in whatever window is already open — + // which may well be an ungoverned one. We can't tell from + // here whether that instance is sandboxed, so refuse and let + // the user restart it deliberately. + return fmt.Errorf("--enforce for %s: it is already running (pid %d). Quit it first, then re-run: macOS would otherwise just focus the existing window instead of launching a sandboxed one", spec.agentName, pid) + } + wrapProfilePath, wrapPort, transparentSess, err = resolveEnforceWrapMode(cfg.Env) + if err != nil { + return fmt.Errorf("enforce mode unavailable: %w", err) + } default: - wrapProfilePath, wrapPort, err = resolveEnforceWrap(cfg.Env) + wrapProfilePath, wrapPort, transparentSess, err = resolveEnforceWrapMode(cfg.Env) if err != nil { return fmt.Errorf("enforce mode unavailable: %w", err) } } + // The pf anchor must not outlive the run: a stale one keeps + // redirecting a group whose listener is gone. The session also + // installs signal handlers for the paths this defer cannot cover. + if transparentSess != nil { + defer func() { _ = transparentSess.Close() }() + } } // Build child environment. @@ -206,8 +244,27 @@ func (c *RunCmd) Run(out *output.Writer) error { // Electron-based agents (e.g. Cursor) ignore embedded user:pass in // HTTPS_PROXY and show a native auth dialog. Inject proxy credentials // into the app's VS Code-style settings.json instead. + // + // NOT under --enforce: there we launch the app ourselves and pass + // Chromium a native --proxy-server pointing at the loopback + // forwarder, which injects the gateway credentials. The app-level + // keys are then both redundant and actively harmful — Electron's + // SimpleURLLoader rejects http.proxyAuthorization with + // net::ERR_INVALID_ARGUMENT, failing every request the editor makes + // (verified: removing the keys took the failures from 4 per launch + // to 0, and the update check completed through the gateway). if a.configDir != "" { - env = injectElectronProxySettings(out, env, a.configDir, caPath) + if guiBinary != "" { + clearElectronProxySettings(out, a.configDir) + // Chromium reads the OS keychain, not our CA env vars, so a + // GUI editor is the one surface where an untrusted or rotated + // CA silently breaks every request. Check before launching: + // the symptom (ERR_CERT_AUTHORITY_INVALID with a correctly + // named CA installed) is very hard to diagnose after the fact. + warnIfGatewayCANotTrusted(out, cfg.CACertificate) + } else { + env = injectElectronProxySettings(out, env, a.configDir, caPath) + } } // Agents with a native proxy config (e.g. Codex) need proxy_url @@ -270,11 +327,31 @@ func (c *RunCmd) Run(out *output.Writer) error { if err != nil { return fmt.Errorf("enforce mode unavailable: %w", err) } - args = enforceWrapArgv(wrapProfilePath, binary, c.Args[1:], agentFramework) + // For a GUI editor, confine the app bundle's own binary rather than + // the CLI launcher (which would exit immediately, leaving the real + // editor running outside the sandbox). Its own args are dropped: + // the launcher's argv (e.g. a path to open) doesn't apply to the + // Electron entrypoint. + sandboxed, sandboxedArgs := binary, c.Args[1:] + if guiBinary != "" { + sandboxed, sandboxedArgs = guiBinary, nil + } + args = enforceWrapArgv(wrapProfilePath, sandboxed, sandboxedArgs, agentFramework, wrapPort) + // Transparent redirect scopes pf by GID, so the confined tree must + // adopt the sandbox group. The helper wraps the LAUNCHER, not the + // other way round: Seatbelt refuses to exec a setgid binary from + // inside the sandbox (verified: execvp Operation not permitted). + if transparentSess != nil { + args = transparentWrapArgv(args) + execBinary = setgidHelperPath + } if notice := enforceWrapNotice(agentFramework); notice != "" { out.Stderr(notice) } out.Stderr(fmt.Sprintf("onecli: enforce mode active — all process egress locked to the gateway (forwarder :%d).", wrapPort)) + if guiBinary != "" { + out.Stderr(fmt.Sprintf("onecli: launching %s inside the sandbox; close it from the app, not this terminal.", filepath.Base(guiBinary))) + } } // Exec — replaces this process so the agent gets direct terminal control. @@ -309,6 +386,25 @@ func writeGatewayCACert(gatewayPEM string) (string, error) { buf.WriteString(gatewayPEM) combined := buf.Bytes() + + // Also keep the bare gateway CA on disk, always in step with the bundle. + // Tools that need a CA they can *install* rather than point an env var at + // (Chromium/Electron reads the OS keychain and ignores SSL_CERT_FILE, so + // governing Cursor's GUI requires `security add-trusted-cert`) need the + // single certificate, not the ~200-root bundle. Writing it here rather + // than on first use is deliberate: a copy that is refreshed on some other + // schedule than the bundle goes stale silently, and a stale CA fails as + // ERR_CERT_AUTHORITY_INVALID with a *correct-looking* subject name, which + // is a genuinely hard error to read. Observed in practice: a months-old + // gateway-ca.pem was trusted in the keychain while the gateway had since + // rotated, so every Chromium request failed even though `security + // find-certificate` showed "OneCLI Local Gateway CA" present and trusted. + // Best-effort: the bundle is what enforce actually depends on, and the + // bare copy only matters for the manual keychain-install step. A failure + // here is surfaced later by warnIfGatewayCANotTrusted, which compares + // against the live CA rather than this file. + _ = writeBareGatewayCA(filepath.Dir(caPath), gatewayPEM) + existing, err := os.ReadFile(caPath) if err == nil && bytes.Equal(existing, combined) { return caPath, nil @@ -319,6 +415,20 @@ func writeGatewayCACert(gatewayPEM string) (string, error) { return caPath, nil } +// writeBareGatewayCA writes just the gateway CA to /gateway-ca.pem. +// Separate from the bundle so it can be installed into an OS trust store. +func writeBareGatewayCA(dir, gatewayPEM string) error { + if strings.TrimSpace(gatewayPEM) == "" { + return nil + } + path := filepath.Join(dir, "gateway-ca.pem") + pem := []byte(gatewayPEM) + if existing, err := os.ReadFile(path); err == nil && bytes.Equal(existing, pem) { + return nil + } + return os.WriteFile(path, pem, 0o600) +} + var systemCAPaths = []string{ "/etc/ssl/cert.pem", // macOS "/etc/ssl/certs/ca-certificates.crt", // Debian/Ubuntu @@ -545,6 +655,7 @@ type agentSpec struct { agentName string baseDir string // home-relative config dir (skills/hooks/plugins live here) configDir string // VS Code-style app dir name; non-empty enables Electron proxy-settings injection. + appBundle string // macOS .app bundle name for GUI editors; under --enforce we exec its binary inside the sandbox instead of the CLI launcher (which only focuses a running app). skipHook bool // true when the gateway hook shouldn't be registered — either the agent has no Claude Code-style hooks (Hermes), or it renders injected hook context visibly in the transcript (Codex), where the auto-loaded onecli-gateway skill carries the same guidance without the noise. pluginGateway bool // true for agents that load the transform_tool_result recovery plugin (e.g. Hermes). dockerSandbox bool // true for agents that run tools in a Docker sandbox needing TERMINAL_DOCKER_* injection. @@ -559,7 +670,17 @@ var supportedAgents = []struct { spec agentSpec }{ {[]string{"claude"}, agentSpec{agentName: "Claude Code", baseDir: ".claude"}}, - {[]string{"cursor", "agent"}, agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor"}}, + // Cursor has two surfaces with OPPOSITE enforce behavior: + // - the GUI launcher (`cursor`, or `agent` when invoked that way) + // only opens/focuses the Electron IDE — no launched process tree to + // sandbox — so configDir routes it to cooperative proxy-settings + // injection and fails --enforce closed (see the enforce switch). + // - the headless agent (`cursor-agent`) IS a launched CLI process, + // exactly like Codex/Claude, so it OMITS configDir and gets the + // real OS-enforced wrap under --enforce. Keeping them as separate + // specs is what lets the same "Cursor" cover both correctly. + {[]string{"cursor", "agent"}, agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor", appBundle: "Cursor"}}, + {[]string{"cursor-agent"}, agentSpec{agentName: "Cursor Agent", baseDir: ".cursor"}}, // Codex skips the hook: it echoes injected hook context into the // transcript (Claude injects it silently), so the hook is pure noise // there. The onecli-gateway skill installed above auto-loads under the @@ -1262,6 +1383,218 @@ func registerUserPromptHook(registrationPath, hookCommand string, includeMatcher return true, nil } +// resolveGUIAppBinary returns the executable inside a GUI editor's macOS +// .app bundle. Under --enforce we must exec THIS, not the `cursor` CLI +// launcher: the launcher just asks macOS to open/focus the app, so +// sandboxing it would confine a process that exits immediately while the +// real editor runs ungoverned. +func resolveGUIAppBinary(spec agentSpec) (string, error) { + if runtime.GOOS != "darwin" { + return "", fmt.Errorf("sandboxed GUI launch is implemented for macOS only (got %s)", runtime.GOOS) + } + if spec.appBundle == "" { + return "", fmt.Errorf("no .app bundle known for %s", spec.agentName) + } + // Search the standard locations; a user-local install is common. + home, _ := os.UserHomeDir() + candidates := []string{ + filepath.Join("/Applications", spec.appBundle+".app", "Contents", "MacOS", spec.appBundle), + } + if home != "" { + candidates = append(candidates, + filepath.Join(home, "Applications", spec.appBundle+".app", "Contents", "MacOS", spec.appBundle)) + } + for _, p := range candidates { + if fi, err := os.Stat(p); err == nil && !fi.IsDir() { + return p, nil + } + } + return "", fmt.Errorf("could not find %s.app (looked in /Applications and ~/Applications)", spec.appBundle) +} + +// guiAlreadyRunning reports whether the GUI editor is already running, and +// its pid. This matters for correctness, not convenience: macOS activates +// the existing instance rather than starting a second one, so launching +// "under the sandbox" while an unsandboxed copy is open would report +// enforcement while the user keeps using an ungoverned editor. +func guiAlreadyRunning(spec agentSpec) (bool, int) { + if runtime.GOOS != "darwin" || spec.appBundle == "" { + return false, 0 + } + out, err := exec.Command("/usr/bin/pgrep", "-x", spec.appBundle).Output() + if err != nil { + return false, 0 // non-zero exit = not running + } + fields := strings.Fields(string(out)) + if len(fields) == 0 { + return false, 0 + } + pid, err := strconv.Atoi(fields[0]) + if err != nil { + return false, 0 + } + return true, pid +} + +// clearElectronProxySettings removes the app-level proxy keys a previous +// (non-enforce) run injected. Under --enforce the editor is launched with +// a native --proxy-server flag instead, and leaving the settings in place +// breaks it two ways: a stale port from an earlier run wins over the flag +// and the sandbox denies it, and http.proxyAuthorization makes Electron's +// SimpleURLLoader reject requests outright with ERR_INVALID_ARGUMENT. +// Only OneCLI's own keys are touched; the rest of settings.json is +// preserved. +func clearElectronProxySettings(out *output.Writer, configDir string) { + path := vscodeSettingsPath(configDir) + if path == "" { + return + } + data, err := os.ReadFile(path) + if err != nil { + return // no settings file: nothing injected, nothing to clear + } + settings := make(map[string]any) + if err := json.Unmarshal(data, &settings); err != nil { + out.Stderr("onecli: warning: could not parse editor settings to clear stale proxy keys; if the editor cannot reach the network, remove http.proxy and http.proxyAuthorization manually") + return + } + changed := false + for _, k := range []string{"http.proxy", "http.proxyAuthorization"} { + if _, present := settings[k]; present { + delete(settings, k) + changed = true + } + } + if !changed { + return + } + encoded, err := json.MarshalIndent(settings, "", " ") + if err != nil { + return + } + if err := os.WriteFile(path, append(encoded, '\n'), 0o600); err != nil { + out.Stderr(fmt.Sprintf("onecli: warning: could not clear stale proxy settings: %v", err)) + return + } + out.Stderr("onecli: cleared app-level proxy settings — under enforce the editor is pointed at the gateway by launch flag instead.") +} + +// warnIfGatewayCANotTrusted checks that the gateway CA the *current* session +// will actually present is the one installed in the login keychain, and says +// exactly how to fix it when it isn't. +// +// This exists because the failure it catches is close to undebuggable from the +// symptom. Chromium/Electron reads the OS keychain and ignores SSL_CERT_FILE +// and NODE_EXTRA_CA_CERTS, so a GUI editor needs the CA installed there. If a +// previously-installed CA has since rotated, the keychain still contains a +// certificate named "OneCLI Local Gateway CA" — `security find-certificate` +// and `dump-trust-settings` both look correct — but its key no longer matches, +// so every request dies as net::ERR_CERT_AUTHORITY_INVALID and the editor's AI +// silently does nothing. Name equality is the trap, so this compares the +// public key, which is what actually has to match. +// +// Warn rather than fail: a stale CA breaks GUI editors, but the sandbox and +// the gateway are still fully enforcing, and CLI agents (which trust the +// bundle by env var) work fine. Refusing to launch would be a worse trade. +func warnIfGatewayCANotTrusted(out *output.Writer, gatewayPEM string) { + if runtime.GOOS != "darwin" || strings.TrimSpace(gatewayPEM) == "" { + return + } + live, err := publicKeyOfPEM([]byte(gatewayPEM)) + if err != nil { + return + } + // Ask the keychain for every cert with this name: a rotation can leave + // several, and the session works if ANY of them is the live one. + cmd := exec.Command("/usr/bin/security", "find-certificate", + "-c", gatewayCACommonName, "-a", "-p") + installed, err := cmd.Output() + if err != nil || len(installed) == 0 { + out.Stderr("onecli: note: the gateway CA is not installed in your login keychain. " + + "GUI editors (Cursor, VS Code) will fail with ERR_CERT_AUTHORITY_INVALID. Install it with:\n" + + " " + gatewayCATrustCommand) + return + } + for _, block := range splitPEMCerts(installed) { + if key, err := publicKeyOfPEM(block); err == nil && bytes.Equal(key, live) { + // Right CA is present. Presence is NOT trust, though: a cert can + // sit in the keychain with no trust settings at all, which is + // exactly what `add-trusted-cert -d` does for a non-root user — + // it writes to the ADMIN domain, silently applies nothing, and + // leaves a certificate that every name-based check reports as + // installed while Chromium still fails the handshake. Confirmed + // on a real machine: `dump-trust-settings` listed only an + // unrelated cert while Cursor logged ERR_CERT_AUTHORITY_INVALID + // on every request. So ask the OS to actually evaluate it. + if gatewayCAIsTrustedByOS() { + return + } + out.Stderr("onecli: warning: the gateway CA is in your keychain but has no trust settings, " + + "so GUI editors will still fail with ERR_CERT_AUTHORITY_INVALID.\n" + + " This is usually the result of using `-d` (admin domain) without root. Fix with:\n" + + " " + gatewayCATrustCommand) + return + } + } + out.Stderr("onecli: warning: a certificate named " + gatewayCACommonName + " is in your keychain, " + + "but it is NOT the CA this gateway is using (the gateway CA has rotated since you trusted it).\n" + + " GUI editors will fail with ERR_CERT_AUTHORITY_INVALID until you re-trust it:\n" + + " security delete-certificate -c \"" + gatewayCACommonName + "\"\n" + + " " + gatewayCATrustCommand) +} + +const gatewayCACommonName = "OneCLI Local Gateway CA" + +// gatewayCATrustCommand is the exact command that works. Note the absence of +// `-d`: that selects the ADMIN trust domain, which needs root. Run as a normal +// user it adds the certificate and applies NO trust settings, producing a +// keychain entry that looks installed and still fails every TLS handshake. +const gatewayCATrustCommand = "security add-trusted-cert -r trustRoot " + + "-k ~/Library/Keychains/login.keychain-db ~/.onecli/gateway-ca.pem" + +// gatewayCAIsTrustedByOS asks macOS whether the gateway CA carries user-domain +// trust settings, rather than inferring trust from the certificate's presence. +// Presence and trust are genuinely different states here, and only the second +// one makes Chromium work. +func gatewayCAIsTrustedByOS() bool { + out, err := exec.Command("/usr/bin/security", "dump-trust-settings").Output() + if err != nil { + // Exits non-zero when the domain holds no trust settings at all, + // which is itself the untrusted answer. + return false + } + return strings.Contains(string(out), gatewayCACommonName) +} + +// publicKeyOfPEM returns a stable encoding of the certificate's public key. +// Comparing the key (not the fingerprint) is deliberate: a CA that is re-issued +// with the same key is still the same trust anchor and should not warn. +func publicKeyOfPEM(pemBytes []byte) ([]byte, error) { + block, _ := pem.Decode(pemBytes) + if block == nil { + return nil, fmt.Errorf("no PEM block") + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, err + } + return x509.MarshalPKIXPublicKey(cert.PublicKey) +} + +// splitPEMCerts splits a concatenated PEM stream into individual blocks. +func splitPEMCerts(data []byte) [][]byte { + var out [][]byte + rest := data + for { + block, remainder := pem.Decode(rest) + if block == nil { + return out + } + out = append(out, pem.EncodeToMemory(block)) + rest = remainder + } +} + // injectElectronProxySettings writes http.proxy and http.proxyAuthorization // into a VS Code-style settings.json so Electron-based editors authenticate // with the gateway proxy without Chromium's native auth dialog. Returns the diff --git a/cmd/onecli/run_enforce_full_chain_live_darwin_test.go b/cmd/onecli/run_enforce_full_chain_live_darwin_test.go new file mode 100644 index 0000000..5412e95 --- /dev/null +++ b/cmd/onecli/run_enforce_full_chain_live_darwin_test.go @@ -0,0 +1,260 @@ +//go:build darwin + +package main + +// THE FULL CHAIN, LIVE. +// +// Everything before this proved pieces: the listener tunnels by SNI (unit + +// live TLS), the pf rules parse and fire (measured: port 80 blackholed at +// 6004ms, port 443 refused at 2ms by the redirect). This joins them. +// +// What it proves: a process that dials a real host DIRECTLY, with no proxy +// configuration of any kind, is captured by pf, routed through our listener, +// and reaches its destination with credentials injected — while a process +// outside the sandbox group is untouched. +// +// That is the mechanism Cursor's extension host needs, end to end. +// +// Requires: setup complete (group, setgid helper, scoped pfctl sudo) and +// ONECLI_LIVE_CHAIN=1. + +import ( + "encoding/base64" + "fmt" + "net" + "os" + "os/exec" + "strings" + "sync" + "testing" + "time" +) + +func liveChainEnabled(t *testing.T) { + t.Helper() + if os.Getenv("ONECLI_LIVE_CHAIN") != "1" { + t.Skip("set ONECLI_LIVE_CHAIN=1 to run the full-chain test") + } + if _, err := os.Stat(setgidHelperPath); err != nil { + t.Skipf("setgid helper not installed: %v", err) + } + if err := verifyTransparentSetup(); err != nil { + t.Skipf("transparent setup incomplete: %v", err) + } +} + +// recordingGateway is a real CONNECT proxy that records what it was asked +// for, so the test can assert the destination survived the trip. +type recordingGateway struct { + ln net.Listener + mu sync.Mutex + seen []string + auth []string +} + +func newRecordingGateway(t *testing.T) *recordingGateway { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("gateway listen: %v", err) + } + g := &recordingGateway{ln: ln} + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + go g.handle(c) + } + }() + t.Cleanup(func() { _ = ln.Close() }) + return g +} + +func (g *recordingGateway) handle(conn net.Conn) { + defer func() { _ = conn.Close() }() + head, err := readHeaderBlock(conn) + if err != nil { + return + } + lines := strings.Split(head, "\r\n") + parts := strings.Fields(lines[0]) + if len(parts) < 2 || parts[0] != "CONNECT" { + return + } + g.mu.Lock() + g.seen = append(g.seen, parts[1]) + for _, l := range lines { + if strings.HasPrefix(strings.ToLower(l), "proxy-authorization:") { + g.auth = append(g.auth, strings.TrimSpace(l[len("proxy-authorization:"):])) + } + } + g.mu.Unlock() + + up, err := net.DialTimeout("tcp", parts[1], 10*time.Second) + if err != nil { + _, _ = conn.Write([]byte("HTTP/1.1 502 Bad Gateway\r\n\r\n")) + return + } + defer func() { _ = up.Close() }() + if _, err := conn.Write([]byte("HTTP/1.1 200 Connection Established\r\n\r\n")); err != nil { + return + } + done := make(chan struct{}) + go func() { _, _ = copyConn(up, conn); close(done) }() + _, _ = copyConn(conn, up) + <-done +} + +func (g *recordingGateway) snapshot() ([]string, []string) { + g.mu.Lock() + defer g.mu.Unlock() + return append([]string(nil), g.seen...), append([]string(nil), g.auth...) +} + +func readHeaderBlock(c net.Conn) (string, error) { + var head []byte + one := make([]byte, 1) + for { + n, err := c.Read(one) + if err != nil { + return "", err + } + if n == 0 { + continue + } + head = append(head, one[0]) + if len(head) >= 4 && string(head[len(head)-4:]) == "\r\n\r\n" { + return string(head), nil + } + if len(head) > 8192 { + return "", fmt.Errorf("header too large") + } + } +} + +func copyConn(dst, src net.Conn) (int64, error) { + buf := make([]byte, 32*1024) + var total int64 + for { + n, err := src.Read(buf) + if n > 0 { + w, werr := dst.Write(buf[:n]) + total += int64(w) + if werr != nil { + return total, werr + } + } + if err != nil { + if tc, ok := dst.(*net.TCPConn); ok { + _ = tc.CloseWrite() + } + return total, err + } + } +} + +// TestLiveFullChain is the demo, as an automated test. +func TestLiveFullChain(t *testing.T) { + liveChainEnabled(t) + + gid, err := sandboxGID(transparentGroupName) + if err != nil { + t.Fatalf("resolving sandbox group: %v", err) + } + + // A real CONNECT proxy standing in for the OneCLI gateway, so the test + // asserts on what the gateway actually receives. + gw := newRecordingGateway(t) + creds := base64.StdEncoding.EncodeToString([]byte("x:aoc_live_chain")) + + // The transparent listener on a fixed port so the pf rule can name it. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listener: %v", err) + } + defer func() { _ = ln.Close() }() + port := uint16(ln.Addr().(*net.TCPAddr).Port) + go serveTransparent(ln, gw.ln.Addr().String(), creds) + + // Load the anchor pointing at this listener. + rules, err := pfRules(port, gid) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + if err := pfLoadAnchor(rules); err != nil { + t.Fatalf("loading anchor (is `sudo pfctl` authorized?): %v", err) + } + t.Cleanup(func() { _ = pfFlushAnchor() }) + + // The subject: curl, run under the sandbox group, with NO proxy + // environment at all. It believes it is dialing example.com directly. + cmd := exec.Command(setgidHelperPath, "curl", "-sS", + "-o", "/dev/null", "-w", "%{http_code}", "--max-time", "20", + "https://example.com/") + cmd.Env = []string{"PATH=/usr/bin:/bin"} // deliberately no *_PROXY vars + out, runErr := cmd.CombinedOutput() + got := strings.TrimSpace(string(out)) + + seen, auth := gw.snapshot() + t.Logf("curl output: %q (err=%v)", got, runErr) + t.Logf("gateway saw CONNECT for: %v", seen) + + if len(seen) == 0 { + t.Fatalf("the gateway saw NO CONNECT: traffic did not traverse the "+ + "transparent listener (curl said %q, err %v)", got, runErr) + } + if seen[0] != "example.com:443" { + t.Fatalf("gateway got CONNECT %q, want example.com:443", seen[0]) + } + if len(auth) == 0 || auth[0] != "Basic "+creds { + t.Fatalf("credentials were not injected: %v", auth) + } + if got != "200" { + t.Fatalf("curl got %q, want 200: the tunnel did not carry the "+ + "request to completion", got) + } + t.Log("FULL CHAIN OK: direct dial -> pf redirect -> SNI recovery -> " + + "credentialed CONNECT -> gateway -> 200, with no proxy config") +} + +// TestLiveFullChainLeavesOthersAlone proves the blast radius is the group. +func TestLiveFullChainLeavesOthersAlone(t *testing.T) { + liveChainEnabled(t) + + gid, err := sandboxGID(transparentGroupName) + if err != nil { + t.Fatalf("resolving sandbox group: %v", err) + } + gw := newRecordingGateway(t) + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listener: %v", err) + } + defer func() { _ = ln.Close() }() + port := uint16(ln.Addr().(*net.TCPAddr).Port) + go serveTransparent(ln, gw.ln.Addr().String(), "dGVzdA==") + + rules, err := pfRules(port, gid) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + if err := pfLoadAnchor(rules); err != nil { + t.Fatalf("loading anchor: %v", err) + } + t.Cleanup(func() { _ = pfFlushAnchor() }) + + // Same request, NOT under the sandbox group: must be untouched. + cmd := exec.Command("curl", "-sS", "-o", "/dev/null", + "-w", "%{http_code}", "--max-time", "20", "https://example.com/") + out, _ := cmd.CombinedOutput() + if got := strings.TrimSpace(string(out)); got != "200" { + t.Fatalf("an ungrouped process got %q; the anchor is affecting "+ + "traffic beyond the sandbox group", got) + } + if seen, _ := gw.snapshot(); len(seen) != 0 { + t.Fatalf("ungrouped traffic was captured by the redirect: %v", seen) + } + t.Log("blast radius confirmed: only the sandbox group is redirected") +} diff --git a/cmd/onecli/run_enforce_pf_darwin.go b/cmd/onecli/run_enforce_pf_darwin.go new file mode 100644 index 0000000..ea4f9c5 --- /dev/null +++ b/cmd/onecli/run_enforce_pf_darwin.go @@ -0,0 +1,379 @@ +//go:build darwin + +package main + +// pf anchor management for transparent-redirect mode (macOS). +// +// The rules answer one question: how do you redirect LOCALLY-ORIGINATED +// traffic to a loopback listener? pf's `rdr` only applies to packets +// arriving on an interface's inbound path, so it never fires for a +// connection this machine originates. The working idiom is two rules: +// +// rdr pass on lo0 inet proto tcp from any to any port 443 -> 127.0.0.1 port P +// pass out route-to lo0 inet proto tcp from any to any port 443 group G keep state +// +// The `route-to lo0` bounces outbound packets onto the loopback INBOUND +// path, where the `rdr` then matches. Without it the rdr is inert — the +// silent-no-op shape this codebase has been bitten by before. +// +// Scoping is by GROUP, not by port or address, and that is the security +// property: only processes running under the dedicated sandbox GID are +// redirected. Everything else on the machine is untouched, so a bug here +// cannot silently capture the user's own browser traffic. +// +// Everything lives in a NAMED anchor. Loading and flushing an anchor leaves +// the main ruleset alone, so we never clobber a user's firewall — a real +// risk given `pfctl -f` on the main ruleset replaces it wholesale. + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "strconv" + "strings" +) + +const ( + // pfAnchorName is the named anchor all OneCLI rules live under. + // + // It is nested under com.apple/ deliberately. macOS's stock ruleset + // contains `anchor "com.apple/*"` and `rdr-anchor "com.apple/*"` but + // nothing that would reach a top-level `onecli` anchor, so rules loaded + // there are accepted by pfctl, visible in its output, and never + // evaluated. Verified on a stock machine: `pfctl -s rules` lists only + // the com.apple anchors. + // + // Nesting here means transparent redirect works with the system's own + // ruleset untouched — no /etc/pf.conf edit, nothing to restore, and + // nothing that a macOS update can revert out from under us. + pfAnchorName = "com.apple/onecli" + // pfctlPath is fixed, never resolved through PATH: a shadowed pfctl + // must not become the enforcement layer (same reasoning as + // sandboxExecPath). + pfctlPath = "/sbin/pfctl" + // sudoPath is likewise fixed. Reading and writing pf state needs root, + // which setup grants through a sudoers entry scoped to pfctl alone. + sudoPath = "/usr/bin/sudo" +) + +// pfMaxGID is the largest GID pf accepts. INT_MAX is reserved as pf's +// "unknown gid" sentinel and is rejected by its parser — established by +// probing the real parser, not by reading the man page. +const pfMaxGID = 2147483646 + +// validateSandboxGID rejects GIDs that pf cannot express or that would +// scope redirection dangerously. +// +// GID 0 (wheel) is refused deliberately: scoping the redirect to it would +// capture root-owned traffic across the whole machine, far beyond the +// sandboxed agent. The whole point of group scoping is that a bug here +// cannot touch anything but the sandbox. +func validateSandboxGID(gid int) error { + if gid <= 0 { + return fmt.Errorf("sandbox GID must be a dedicated non-root group, got %d", gid) + } + if gid > pfMaxGID { + return fmt.Errorf("sandbox GID %d exceeds the maximum pf accepts (%d)", gid, pfMaxGID) + } + return nil +} + +// pfRules renders the anchor body for a forwarder port and sandbox GID. +// Returns an error rather than emitting rules that pf would reject at load +// time, when a partial failure is far more expensive to diagnose. +// +// Rule order matters and is not cosmetic. pf uses LAST-MATCH semantics for +// filter rules, so the default-deny is written first and the narrow allow +// second; reversing them would silently drop everything. +// +// The `block drop out ... group G` line is the security keystone. Because +// Seatbelt must now permit outbound 443 for pf to have a packet to redirect +// (measured: Seatbelt denies at connect(), before any packet exists, so a +// Seatbelt-denied connection never reaches pf), the OS-level deny that used +// to be our fail-closed guarantee is gone for that port. Without this line +// the design would be fail-OPEN: an anchor that is missing, flushed, or +// unreferenced would let the agent dial the internet directly and silently. +// +// With it, the anchor denies ALL egress for the sandbox group and then +// re-permits exactly one path: redirected 443. Loopback is allowed so the +// redirect itself, and local dev servers, keep working. +func pfRules(port uint16, gid int) (string, error) { + if err := validateSandboxGID(gid); err != nil { + return "", err + } + if port == 0 { + return "", fmt.Errorf("forwarder port is unset") + } + var b strings.Builder + // NAT: rewrite the redirected destination to our listener. + fmt.Fprintf(&b, + "rdr pass on lo0 inet proto tcp from any to any port %d -> 127.0.0.1 port %d\n", + transparentRedirectPort, port) + + // Filter, in last-match order. + // 1. Default-deny every protocol for the sandbox group. + fmt.Fprintf(&b, "block drop out inet from any to any group %d\n", gid) + fmt.Fprintf(&b, "block drop out inet6 from any to any group %d\n", gid) + // 2. Loopback stays open: the redirect lands there, and local dev + // servers must keep working. + fmt.Fprintf(&b, "pass out on lo0 inet from any to any group %d keep state\n", gid) + // 3. The one sanctioned egress path: 443, diverted to the listener. + fmt.Fprintf(&b, + "pass out route-to lo0 inet proto tcp from any to any port %d group %d keep state\n", + transparentRedirectPort, gid) + // 4. DNS stays permitted: name resolution happens in the process + // itself, and without it the SNI we depend on is never produced. + // UDP/53 carries no payload we govern, and the gateway still + // adjudicates every connection that follows. + fmt.Fprintf(&b, "pass out inet proto udp from any to any port 53 group %d keep state\n", gid) + return b.String(), nil +} + +// pfSudoWorks reports whether we can actually run pfctl under sudo without +// a password. +// +// Probes the capability rather than reading /etc/sudoers.d: that file is +// mode 440 root:wheel and unreadable by the user, so a read-based check +// reports MISSING even when the entry is present and working. Checking what +// we can DO is both accurate and the thing we care about. +func pfSudoWorks() bool { + return pfctlCommand("-s", "info").Run() == nil +} + +// pfAvailable reports whether pfctl exists and is usable. +func pfAvailable() error { + if _, err := os.Stat(pfctlPath); err != nil { + return fmt.Errorf("%s not found — cannot install transparent redirect", pfctlPath) + } + return nil +} + +// pfctlCommand builds a privileged pfctl invocation. +// +// Every pf operation, including reads, needs root: /dev/pf is root-only. +// Setup grants this through a sudoers entry scoped to pfctl alone, so this +// is not blanket privilege. +// +// -n (non-interactive) is deliberate: if the sudoers entry is missing we +// want an immediate, diagnosable failure rather than a password prompt +// blocking a background process forever. +func pfctlCommand(args ...string) *exec.Cmd { + full := append([]string{"-n", pfctlPath}, args...) + return exec.Command(sudoPath, full...) +} + +// pfValidateRules syntax-checks a ruleset WITHOUT root and WITHOUT loading +// it. Called before any privileged operation so a malformed rule surfaces +// as a clear error rather than a half-applied firewall change. +func pfValidateRules(rules string) error { + cmd := exec.Command(pfctlPath, "-n", "-f", "-") + cmd.Stdin = strings.NewReader(rules) + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("pf rules rejected: %s", pfCleanOutput(out)) + } + return nil +} + +// pfCleanOutput strips pfctl's unconditional main-ruleset warning so real +// errors are readable. +func pfCleanOutput(b []byte) string { + var keep []string + for _, l := range strings.Split(string(b), "\n") { + t := strings.TrimSpace(l) + if t == "" || + strings.Contains(t, "could result in flushing") || + strings.Contains(t, "present in the main ruleset") || + strings.Contains(t, "See /etc/pf.conf") { + continue + } + keep = append(keep, t) + } + return strings.Join(keep, "; ") +} + +// pfEnabled reports whether the packet filter is currently active. Loading +// an anchor into a disabled pf silently does nothing — the exact silent-no-op +// failure mode this codebase guards against elsewhere, so the caller must +// check rather than assume. +func pfEnabled() (bool, error) { + out, err := pfctlCommand("-s", "info").CombinedOutput() + if err != nil { + return false, fmt.Errorf("querying pf status: %s", pfCleanOutput(out)) + } + return strings.Contains(string(out), "Status: Enabled"), nil +} + +// pfAnchorLoaded returns the rules currently in our anchor, so callers can +// verify what is actually installed instead of trusting a prior write. +func pfAnchorLoaded() (string, error) { + nat, err := pfctlStdout("-a", pfAnchorName, "-s", "nat") + if err != nil { + return "", fmt.Errorf("reading anchor: %w", err) + } + rules, err := pfctlStdout("-a", pfAnchorName, "-s", "rules") + if err != nil { + return "", fmt.Errorf("reading anchor rules: %w", err) + } + return nat + rules, nil +} + +// pfctlStdout runs pfctl and returns STDOUT ONLY. +// +// This distinction is load-bearing, not tidiness. pfctl unconditionally +// writes to stderr on this kernel: +// +// No ALTQ support in kernel +// ALTQ related functions disabled +// +// CombinedOutput() folds those two lines into the result, so an EMPTY +// anchor read back as four nonblank lines. Any caller counting rules, or +// checking whether the anchor holds anything, silently saw a populated +// anchor when it was empty — including verifyLoaded, whose entire job is +// catching that case. +func pfctlStdout(args ...string) (string, error) { + full := append([]string{"-n", pfctlPath}, args...) + cmd := exec.Command(sudoPath, full...) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return "", fmt.Errorf("%s", pfCleanOutput(stderr.Bytes())) + } + return stdout.String(), nil +} + +// pfEnable turns the packet filter on and returns the reference token +// macOS hands back. +// +// pf on macOS is REFERENCE COUNTED: `pfctl -E` increments a counter and +// returns a token, and pf stays up only while at least one reference is +// held. Observed live: pf was enabled during setup, then found Disabled +// later in the same session, which made an enforced run refuse to start +// with "pf is disabled". Enabling it once at setup is therefore not +// durable, so an enforced run enables it for itself. +// +// The token is deliberately NOT released on exit. Releasing it would turn +// pf off for anything else relying on it, and leaving it held is the +// conservative choice: pf up with an empty anchor governs nothing, while pf +// down would silently ungovern a concurrent enforced run. +func pfEnable() error { + out, err := pfctlCommand("-E").CombinedOutput() + if err != nil { + return fmt.Errorf("enabling pf: %s", pfCleanOutput(out)) + } + return nil +} + +// pfEnsureEnabled turns pf on if it is off, so a run never fails merely +// because a reference elsewhere was dropped. +func pfEnsureEnabled() error { + on, err := pfEnabled() + if err != nil { + return err + } + if on { + return nil + } + if err := pfEnable(); err != nil { + return err + } + // Verify rather than trust: `pfctl -E` can report success while pf + // stays down if another reference was released concurrently. + on, err = pfEnabled() + if err != nil { + return err + } + if !on { + return fmt.Errorf("pf did not stay enabled; its rules would never evaluate") + } + return nil +} + +// pfLoadAnchor installs the rules into the named anchor. Requires root. +// +// Returns a descriptive error on failure rather than degrading silently: +// --enforce promises enforcement, and a transparent redirect that did not +// install means traffic the caller believes is governed is not. +func pfLoadAnchor(rules string) error { + if err := pfValidateRules(rules); err != nil { + return err + } + cmd := pfctlCommand("-a", pfAnchorName, "-f", "-") + cmd.Stdin = strings.NewReader(rules) + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("loading pf anchor (needs root): %s", pfCleanOutput(out)) + } + return nil +} + +// pfFlushAnchor removes our rules, leaving the rest of pf untouched. +func pfFlushAnchor() error { + out, err := pfctlCommand("-a", pfAnchorName, "-F", "all").CombinedOutput() + if err != nil { + return fmt.Errorf("flushing pf anchor: %s", pfCleanOutput(out)) + } + return nil +} + +// pfMainRulesetReferencesAnchor reports whether the system ruleset actually +// dispatches into our anchor. An anchor that nothing references holds rules +// that never evaluate — loaded, visible in pfctl output, and completely +// inert. Checking is the difference between "installed" and "working". +func pfMainRulesetReferencesAnchor() (bool, error) { + natOut, err := pfctlStdout("-s", "nat") + if err != nil { + return false, fmt.Errorf("reading main nat ruleset: %w", err) + } + ruleOut, err := pfctlStdout("-s", "rules") + if err != nil { + return false, fmt.Errorf("reading main ruleset: %w", err) + } + combined := natOut + ruleOut + return anchorIsReachable(combined, pfAnchorName), nil +} + +// anchorIsReachable reports whether a main ruleset dispatches into the +// named anchor. +// +// Nesting is the subtle part. Our anchor is "com.apple/onecli", and macOS's +// stock ruleset contains `anchor "com.apple/*"`. The wildcard covers every +// child, so an exact-name search would wrongly report the anchor as +// unreachable and refuse to run — which is exactly what happened before +// this function existed. A parent wildcard is a real reference and must +// count as one. +func anchorIsReachable(ruleset, anchor string) bool { + if strings.Contains(ruleset, `"`+anchor+`"`) { + return true + } + // Walk parent prefixes: com.apple/onecli -> com.apple/*, then */*. + parts := strings.Split(anchor, "/") + for i := len(parts) - 1; i > 0; i-- { + prefix := strings.Join(parts[:i], "/") + if strings.Contains(ruleset, `"`+prefix+`/*"`) { + return true + } + } + return strings.Contains(ruleset, `anchor "*"`) +} + +// sandboxGID resolves the dedicated group used to scope redirection. +// Returns the numeric GID. +func sandboxGID(groupName string) (int, error) { + out, err := exec.Command("/usr/bin/dscl", ".", "-read", + "/Groups/"+groupName, "PrimaryGroupID").CombinedOutput() + if err != nil { + return 0, fmt.Errorf("group %q not found", groupName) + } + fields := strings.Fields(string(out)) + if len(fields) < 2 { + return 0, fmt.Errorf("unexpected dscl output for group %q", groupName) + } + gid, err := strconv.Atoi(fields[len(fields)-1]) + if err != nil { + return 0, fmt.Errorf("parsing GID for %q: %w", groupName, err) + } + return gid, nil +} diff --git a/cmd/onecli/run_enforce_pf_darwin_test.go b/cmd/onecli/run_enforce_pf_darwin_test.go new file mode 100644 index 0000000..3057280 --- /dev/null +++ b/cmd/onecli/run_enforce_pf_darwin_test.go @@ -0,0 +1,248 @@ +//go:build darwin + +package main + +// Tests for the pf anchor layer. Everything here runs WITHOUT root: rule +// rendering and syntax validation are the parts that must be right before +// any privileged operation happens, and `pfctl -n` validates syntax +// unprivileged. + +import ( + "strings" + "testing" +) + +// TestPFRulesAreValidSyntax is the load-bearing test: it feeds our generated +// rules to the real pfctl parser. A hand-written assertion on the string +// would only prove the rules match my expectations, not that the kernel's +// parser accepts them — the same class of mistake as a Seatbelt rule that +// loads cleanly and matches nothing. +func TestPFRulesAreValidSyntax(t *testing.T) { + if err := pfAvailable(); err != nil { + t.Skipf("pfctl unavailable: %v", err) + } + for _, tc := range []struct { + port uint16 + gid int + }{ + {8080, 5000}, + {1, 1}, + {65535, pfMaxGID}, + {443, 20}, + } { + rules, err := pfRules(tc.port, tc.gid) + if err != nil { + t.Fatalf("pfRules(%d, %d): %v", tc.port, tc.gid, err) + } + if err := pfValidateRules(rules); err != nil { + t.Fatalf("pfRules(%d, %d) produced invalid syntax: %v\n%s", + tc.port, tc.gid, err, rules) + } + } +} + +// TestPFValidateRulesRejectsGarbage proves the validator actually validates. +// Without this, TestPFRulesAreValidSyntax could pass against a checker that +// accepts anything. +func TestPFValidateRulesRejectsGarbage(t *testing.T) { + if err := pfAvailable(); err != nil { + t.Skipf("pfctl unavailable: %v", err) + } + for name, rules := range map[string]string{ + "prose": "this is not a pf rule\n", + "bad keyword": "rdrr pass on lo0 -> 127.0.0.1\n", + "missing target": "rdr pass on lo0 inet proto tcp from any to any port 443 ->\n", + "bad port": "rdr pass on lo0 inet proto tcp from any to any port notaport -> 127.0.0.1 port 8080\n", + } { + t.Run(name, func(t *testing.T) { + if err := pfValidateRules(rules); err == nil { + t.Fatalf("validator accepted invalid rules: %q", rules) + } + }) + } +} + +// TestPFRulesScopeToGroup: redirection must be scoped to the sandbox GID. +// An unscoped rule would capture the user's own traffic — their browser, +// their mail client — which is both a privacy problem and a support +// nightmare. +func TestPFRulesScopeToGroup(t *testing.T) { + rules, err := pfRules(9999, 5000) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + if !strings.Contains(rules, "group 5000") { + t.Fatalf("rules are not scoped to the sandbox group:\n%s", rules) + } + // EVERY filter rule must carry the group. One unscoped line would + // apply the policy machine-wide. + for _, l := range strings.Split(rules, "\n") { + l = strings.TrimSpace(l) + if l == "" || strings.HasPrefix(l, "rdr ") { + continue + } + if !strings.Contains(l, "group 5000") { + t.Fatalf("unscoped filter rule would apply machine-wide: %q", l) + } + } + // The pass rule is the one that diverts traffic; it MUST carry the + // group. Check the specific line rather than the blob. + var passLine string + for _, l := range strings.Split(rules, "\n") { + if strings.HasPrefix(l, "pass out") { + passLine = l + } + } + if passLine == "" { + t.Fatal("no `pass out` rule rendered") + } + if !strings.Contains(passLine, "group 5000") { + t.Fatalf("the diverting rule is unscoped: %q", passLine) + } +} + +// TestPFRulesRedirectOnly443 documents and enforces the port scope. +// Redirecting all ports would catch protocols whose destination we cannot +// recover from an SNI, silently breaking them. +func TestPFRulesRedirectOnly443(t *testing.T) { + rules, err := pfRules(9999, 5000) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + if strings.Contains(rules, "port = any") || strings.Contains(rules, "to any port any") { + t.Fatalf("rules redirect more than 443:\n%s", rules) + } + if strings.Count(rules, "port 443") < 2 { + t.Fatalf("expected the rdr and pass rules to scope to 443:\n%s", rules) + } +} + +// TestPFRulesUseRouteTo guards the subtle part. A plain `rdr` without +// `route-to lo0` loads cleanly, appears in pfctl output, and never fires for +// locally-originated traffic. That silent no-op is exactly the failure mode +// this project keeps hitting, so it is asserted rather than trusted. +func TestPFRulesUseRouteTo(t *testing.T) { + rules, err := pfRules(9999, 5000) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + if !strings.Contains(rules, "route-to lo0") { + t.Fatalf("missing `route-to lo0`; the rdr will never fire for local traffic:\n%s", rules) + } +} + +func TestPFCleanOutputStripsBoilerplate(t *testing.T) { + raw := []byte("pfctl: Use of -f option, could result in flushing of rules\n" + + "present in the main ruleset added by the system at startup.\n" + + "See /etc/pf.conf for further details.\n" + + "stdin:1: syntax error\n") + got := pfCleanOutput(raw) + if got != "stdin:1: syntax error" { + t.Fatalf("pfCleanOutput = %q, want the real error only", got) + } +} + +// TestPFRulesRejectBadGID: pf reserves INT_MAX as its "unknown gid" +// sentinel and rejects it at parse time. GID 0 is refused by us because +// scoping the redirect to wheel would capture root traffic machine-wide. +// Both are caught before any rule is emitted, discovered by probing the +// real parser rather than trusting documentation. +func TestPFRulesRejectBadGID(t *testing.T) { + for name, gid := range map[string]int{ + "root/wheel": 0, + "negative": -1, + "pf sentinel": 2147483647, + "above max": 4294967295, + } { + t.Run(name, func(t *testing.T) { + if _, err := pfRules(8080, gid); err == nil { + t.Fatalf("pfRules accepted an unusable GID %d", gid) + } + }) + } +} + +// TestPFRulesRejectZeroPort: an unset port would render a rule redirecting +// to port 0, which loads but blackholes traffic. +func TestPFRulesRejectZeroPort(t *testing.T) { + if _, err := pfRules(0, 5000); err == nil { + t.Fatal("pfRules accepted a zero forwarder port") + } +} + +// TestPFRulesAreFailClosed is the most important test in this file. +// +// Transparent redirect forces the Seatbelt profile to ALLOW outbound 443, +// because Seatbelt adjudicates at connect() before a packet exists and pf +// can only redirect packets that exist (measured: a denied connect returns +// EPERM in 16ms while an allowed one takes the full 6s network timeout). +// That removes the OS-level deny which used to be the fail-closed +// guarantee, so the anchor itself must deny by default. +// +// Without a default-deny, a missing or flushed anchor means direct, +// silent, ungoverned egress. This test is what stops that shipping. +func TestPFRulesAreFailClosed(t *testing.T) { + rules, err := pfRules(9999, 5000) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + if !strings.Contains(rules, "block drop out inet from any to any group 5000") { + t.Fatalf("no IPv4 default-deny; a flushed anchor would be fail-OPEN:\n%s", rules) + } + if !strings.Contains(rules, "block drop out inet6 from any to any group 5000") { + t.Fatalf("no IPv6 default-deny; v6 egress would bypass governance:\n%s", rules) + } + + // pf is LAST-match for filter rules: the deny must precede the allows. + lines := strings.Split(strings.TrimSpace(rules), "\n") + denyIdx, passIdx := -1, -1 + for i, l := range lines { + if strings.HasPrefix(l, "block drop out inet ") && denyIdx == -1 { + denyIdx = i + } + if strings.Contains(l, "route-to lo0") { + passIdx = i + } + } + if denyIdx == -1 || passIdx == -1 { + t.Fatalf("expected both a default-deny and a redirect rule:\n%s", rules) + } + if denyIdx > passIdx { + t.Fatalf("default-deny at %d comes AFTER the allow at %d; last-match "+ + "semantics would drop all traffic:\n%s", denyIdx, passIdx, rules) + } +} + +// TestPFRulesPermitDNS: name resolution happens in the sandboxed process, +// and the SNI we route by does not exist without it. +func TestPFRulesPermitDNS(t *testing.T) { + rules, err := pfRules(9999, 5000) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + if !strings.Contains(rules, "port 53") { + t.Fatalf("DNS is blocked; the sandbox cannot resolve names:\n%s", rules) + } +} + +// TestPFRulesDenyArbitraryPorts: only 443 and DNS are permitted outbound. +// A rule permitting, say, 22 or 8080 would be an ungoverned egress channel. +func TestPFRulesDenyArbitraryPorts(t *testing.T) { + rules, err := pfRules(9999, 5000) + if err != nil { + t.Fatalf("pfRules: %v", err) + } + for _, l := range strings.Split(rules, "\n") { + l = strings.TrimSpace(l) + if !strings.HasPrefix(l, "pass out") { + continue + } + // Permitted: loopback (any port), 443, DNS. + if strings.Contains(l, "on lo0") || + strings.Contains(l, "port 443") || + strings.Contains(l, "port 53") { + continue + } + t.Fatalf("rule permits egress beyond 443/DNS/loopback: %q", l) + } +} diff --git a/cmd/onecli/run_enforce_sni.go b/cmd/onecli/run_enforce_sni.go new file mode 100644 index 0000000..e53dfd0 --- /dev/null +++ b/cmd/onecli/run_enforce_sni.go @@ -0,0 +1,264 @@ +package main + +// TLS ClientHello SNI extraction for transparent-redirect mode. +// +// Why this exists: under transparent redirection an app dials a real host +// directly and pf rewrites the destination to our loopback listener. The +// connection therefore arrives with NO protocol-level statement of where it +// was going — the original destination lives only in pf's state table. +// +// The obvious way to recover it is the DIOCNATLOOK ioctl on the pf device, +// but that requires root on EVERY connection. Parsing the SNI instead needs +// no privilege at all, and the hostname is exactly what the gateway's +// CONNECT needs. Root stays confined to loading the pf anchor once at +// session start, never in the data path. +// +// Deliberately hand-rolled rather than using crypto/tls: we must inspect the +// ClientHello WITHOUT terminating TLS (the gateway does that), and Go's +// stdlib offers no "peek at the handshake" primitive that leaves the bytes +// replayable. +// +// Fails CLOSED: any malformed, truncated, non-TLS, or SNI-less hello returns +// an error and the connection is refused. Guessing a destination would mean +// sending an agent's traffic somewhere it did not ask for. + +import ( + "errors" + "fmt" + "strings" +) + +var ( + errNotTLS = errors.New("not a TLS ClientHello") + errNoSNI = errors.New("ClientHello carries no SNI extension") + errHelloTooBig = errors.New("ClientHello exceeds the maximum record size") + errHelloPartial = errors.New("ClientHello is incomplete") +) + +const ( + // TLS record layer. + tlsRecordTypeHandshake = 0x16 + tlsHandshakeClientHello = 0x01 + tlsExtensionServerName = 0x0000 + sniTypeHostName = 0x00 + + // A ClientHello must fit one record. 16KB is the TLS maximum plaintext + // fragment; anything larger is not a hello we can use. + maxClientHelloSize = 16 * 1024 + // Minimum bytes needed before the record length is knowable. + tlsRecordHeaderLen = 5 +) + +// parseSNI extracts the server_name from a complete TLS ClientHello. +// +// The input must begin at the record header. Returns errHelloPartial if buf +// does not yet hold the whole record, which lets the caller read more rather +// than fail — the hello can legitimately arrive split across TCP segments. +func parseSNI(buf []byte) (string, error) { + if len(buf) < tlsRecordHeaderLen { + return "", errHelloPartial + } + if buf[0] != tlsRecordTypeHandshake { + // Not a handshake record. Could be plain HTTP or another protocol; + // either way we cannot recover a destination from it. + return "", errNotTLS + } + // buf[1:3] is the record-layer version. Deliberately NOT validated: + // TLS 1.3 sends a legacy 0x0303 here and middleboxes vary. The + // handshake type below is the reliable discriminator. + recordLen := int(buf[3])<<8 | int(buf[4]) + if recordLen > maxClientHelloSize { + return "", errHelloTooBig + } + if len(buf) < tlsRecordHeaderLen+recordLen { + return "", errHelloPartial + } + body := buf[tlsRecordHeaderLen : tlsRecordHeaderLen+recordLen] + + return parseClientHelloBody(body) +} + +// parseClientHelloBody walks the handshake message. Every step is +// length-checked: this parses attacker-influenced bytes from a process we +// are sandboxing precisely because we do not trust it. +func parseClientHelloBody(b []byte) (string, error) { + r := &byteReader{buf: b} + + msgType, ok := r.u8() + if !ok { + return "", errHelloPartial + } + if msgType != tlsHandshakeClientHello { + return "", errNotTLS + } + msgLen, ok := r.u24() + if !ok { + return "", errHelloPartial + } + // The handshake message may be shorter than the record (multiple + // messages per record is legal); clamp to it. + body, ok := r.take(int(msgLen)) + if !ok { + return "", errHelloPartial + } + r = &byteReader{buf: body} + + if _, ok := r.take(2); !ok { // client_version + return "", errHelloPartial + } + if _, ok := r.take(32); !ok { // random + return "", errHelloPartial + } + if _, ok := r.vector8(); !ok { // legacy_session_id + return "", errHelloPartial + } + if _, ok := r.vector16(); !ok { // cipher_suites + return "", errHelloPartial + } + if _, ok := r.vector8(); !ok { // compression_methods + return "", errHelloPartial + } + + exts, ok := r.vector16() + if !ok { + // No extensions block at all: legal in ancient TLS, but then there + // is no SNI and we cannot route. + return "", errNoSNI + } + er := &byteReader{buf: exts} + for er.remaining() > 0 { + extType, ok := er.u16() + if !ok { + return "", errHelloPartial + } + extData, ok := er.vector16() + if !ok { + return "", errHelloPartial + } + if extType != tlsExtensionServerName { + continue + } + return parseServerNameExtension(extData) + } + return "", errNoSNI +} + +// parseServerNameExtension reads a ServerNameList and returns the first +// host_name entry. +func parseServerNameExtension(b []byte) (string, error) { + r := &byteReader{buf: b} + list, ok := r.vector16() + if !ok { + return "", errHelloPartial + } + lr := &byteReader{buf: list} + for lr.remaining() > 0 { + nameType, ok := lr.u8() + if !ok { + return "", errHelloPartial + } + name, ok := lr.vector16() + if !ok { + return "", errHelloPartial + } + if nameType != sniTypeHostName { + continue + } + host := string(name) + if err := validateSNIHost(host); err != nil { + return "", err + } + return host, nil + } + return "", errNoSNI +} + +// validateSNIHost rejects hostnames we refuse to put in a CONNECT line. +// +// This is a security boundary, not hygiene: the value is attacker-controlled +// and gets interpolated into an HTTP request line sent upstream. A embedded +// CRLF would let a sandboxed process forge arbitrary headers on the gateway +// connection — including its own Proxy-Authorization. Fail closed on +// anything that is not a plausible DNS name. +func validateSNIHost(h string) error { + if h == "" { + return errNoSNI + } + if len(h) > 253 { + return fmt.Errorf("SNI host exceeds the DNS length limit") + } + for i := 0; i < len(h); i++ { + c := h[i] + isAlnum := (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') + if isAlnum || c == '-' || c == '.' || c == '_' { + continue + } + return fmt.Errorf("SNI host contains an illegal byte %q", c) + } + // A leading dot or a doubled dot is not a resolvable name and suggests + // a crafted value. + if strings.HasPrefix(h, ".") || strings.Contains(h, "..") { + return fmt.Errorf("SNI host is not a well-formed DNS name") + } + return nil +} + +// byteReader is a bounds-checked cursor. Every read returns ok=false rather +// than panicking, so malformed input becomes a refused connection. +type byteReader struct { + buf []byte + pos int +} + +func (r *byteReader) remaining() int { return len(r.buf) - r.pos } + +func (r *byteReader) take(n int) ([]byte, bool) { + if n < 0 || r.remaining() < n { + return nil, false + } + out := r.buf[r.pos : r.pos+n] + r.pos += n + return out, true +} + +func (r *byteReader) u8() (uint8, bool) { + b, ok := r.take(1) + if !ok { + return 0, false + } + return b[0], true +} + +func (r *byteReader) u16() (uint16, bool) { + b, ok := r.take(2) + if !ok { + return 0, false + } + return uint16(b[0])<<8 | uint16(b[1]), true +} + +func (r *byteReader) u24() (uint32, bool) { + b, ok := r.take(3) + if !ok { + return 0, false + } + return uint32(b[0])<<16 | uint32(b[1])<<8 | uint32(b[2]), true +} + +// vector8 reads a length-prefixed vector with a 1-byte length. +func (r *byteReader) vector8() ([]byte, bool) { + n, ok := r.u8() + if !ok { + return nil, false + } + return r.take(int(n)) +} + +// vector16 reads a length-prefixed vector with a 2-byte length. +func (r *byteReader) vector16() ([]byte, bool) { + n, ok := r.u16() + if !ok { + return nil, false + } + return r.take(int(n)) +} diff --git a/cmd/onecli/run_enforce_sni_test.go b/cmd/onecli/run_enforce_sni_test.go new file mode 100644 index 0000000..1934ef7 --- /dev/null +++ b/cmd/onecli/run_enforce_sni_test.go @@ -0,0 +1,189 @@ +package main + +// Tests for SNI extraction. The critical one is +// TestParseSNIAgainstRealClientHello: it feeds bytes produced by Go's own +// crypto/tls, so the parser is validated against a real implementation +// rather than a hand-built fixture that could share my misreading of the +// spec. + +import ( + "crypto/tls" + "errors" + "net" + "strings" + "testing" + "time" +) + +// captureClientHello starts a listener, points a real TLS client at it, and +// returns the raw first flight the client sent. +func captureClientHello(t *testing.T, serverName string) []byte { + t.Helper() + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer func() { _ = ln.Close() }() + + type result struct { + buf []byte + err error + } + done := make(chan result, 1) + + go func() { + conn, err := ln.Accept() + if err != nil { + done <- result{err: err} + return + } + defer func() { _ = conn.Close() }() + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + buf := make([]byte, 4096) + n, err := conn.Read(buf) + if err != nil { + done <- result{err: err} + return + } + done <- result{buf: buf[:n]} + }() + + // The handshake will fail (no server cert); we only need the hello. + c, err := net.DialTimeout("tcp", ln.Addr().String(), 5*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + tlsConn := tls.Client(c, &tls.Config{ServerName: serverName}) + _ = tlsConn.SetDeadline(time.Now().Add(2 * time.Second)) + _ = tlsConn.Handshake() // expected to fail + _ = tlsConn.Close() + + r := <-done + if r.err != nil { + t.Fatalf("capturing hello: %v", r.err) + } + return r.buf +} + +func TestParseSNIAgainstRealClientHello(t *testing.T) { + for _, host := range []string{ + "api.anthropic.com", + "agentn.global.api5.cursor.sh", + "a.co", + // Long-but-legal name: exercises multi-byte vector lengths. + strings.Repeat("sub.", 40) + "example.com", + } { + t.Run(host, func(t *testing.T) { + hello := captureClientHello(t, host) + got, err := parseSNI(hello) + if err != nil { + t.Fatalf("parseSNI on a real hello for %q: %v", host, err) + } + if got != host { + t.Fatalf("parseSNI = %q, want %q", got, host) + } + }) + } +} + +// TestParseSNIPartialRecord feeds the hello one byte at a time and requires +// errHelloPartial until the record is complete. A parser that guessed early +// would route traffic based on a truncated name. +func TestParseSNIPartialRecord(t *testing.T) { + const host = "api.anthropic.com" + hello := captureClientHello(t, host) + + for n := 0; n < len(hello); n++ { + _, err := parseSNI(hello[:n]) + if !errors.Is(err, errHelloPartial) { + t.Fatalf("parseSNI on %d/%d bytes: got %v, want errHelloPartial", + n, len(hello), err) + } + } + got, err := parseSNI(hello) + if err != nil || got != host { + t.Fatalf("parseSNI on the complete hello = %q, %v", got, err) + } +} + +func TestParseSNIRejectsNonTLS(t *testing.T) { + for name, input := range map[string][]byte{ + "http request": []byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n"), + "ssh banner": []byte("SSH-2.0-OpenSSH_9.0\r\n"), + "random binary": {0xde, 0xad, 0xbe, 0xef, 0x00, 0x11, 0x22}, + } { + t.Run(name, func(t *testing.T) { + if _, err := parseSNI(input); !errors.Is(err, errNotTLS) { + t.Fatalf("got %v, want errNotTLS", err) + } + }) + } +} + +// TestParseSNIRejectsHeaderInjection is the security-critical case: the SNI +// is attacker-controlled and ends up in a CONNECT request line. A CRLF must +// never survive parsing. +func TestParseSNIRejectsHeaderInjection(t *testing.T) { + for name, host := range map[string]string{ + "crlf": "evil.com\r\nX-Injected: 1", + "lf only": "evil.com\nX-Injected: 1", + "space": "evil.com X-Injected", + "null byte": "evil.com\x00", + "colon port": "evil.com:1234", + "slash path": "evil.com/path", + "leading dot": ".evil.com", + "double dot": "evil..com", + } { + t.Run(name, func(t *testing.T) { + if err := validateSNIHost(host); err == nil { + t.Fatalf("validateSNIHost(%q) accepted an unsafe hostname", host) + } + }) + } +} + +func TestValidateSNIHostAcceptsRealNames(t *testing.T) { + for _, host := range []string{ + "api.anthropic.com", + "agentn.global.api5.cursor.sh", + "a.co", + "my-host_1.example.com", + "localhost", + } { + if err := validateSNIHost(host); err != nil { + t.Fatalf("validateSNIHost(%q) rejected a legitimate name: %v", host, err) + } + } +} + +// TestParseSNIFuzzDoesNotPanic feeds truncations and byte-flips of a real +// hello. The parser must always return an error, never panic: it runs on +// input from the very process we are sandboxing. +func TestParseSNIFuzzDoesNotPanic(t *testing.T) { + hello := captureClientHello(t, "api.anthropic.com") + + for i := 0; i < len(hello); i++ { + for _, flip := range []byte{0x00, 0xff, 0x7f} { + mutated := make([]byte, len(hello)) + copy(mutated, hello) + mutated[i] = flip + func() { + defer func() { + if r := recover(); r != nil { + t.Fatalf("panic on byte %d flipped to %#x: %v", i, flip, r) + } + }() + _, _ = parseSNI(mutated) + }() + } + } +} + +func TestParseSNIRejectsOversizedRecord(t *testing.T) { + // Record header claiming a length beyond the TLS maximum. + buf := []byte{tlsRecordTypeHandshake, 0x03, 0x03, 0xff, 0xff} + if _, err := parseSNI(buf); !errors.Is(err, errHelloTooBig) { + t.Fatalf("got %v, want errHelloTooBig", err) + } +} diff --git a/cmd/onecli/run_enforce_transparent.go b/cmd/onecli/run_enforce_transparent.go new file mode 100644 index 0000000..670ecc5 --- /dev/null +++ b/cmd/onecli/run_enforce_transparent.go @@ -0,0 +1,193 @@ +package main + +// Transparent-redirect listener for enforce mode. +// +// The problem it solves: some applications dial remote hosts DIRECTLY, +// ignoring every proxy mechanism we can set from outside — env vars +// (HTTPS_PROXY, NODE_USE_ENV_PROXY), Chromium's --proxy-server, and the +// editor's own settings. Cursor's extension host is the observed case; the +// kernel names it plainly: +// +// Sandbox: Cursor Helper (Plugin)(81300) deny(1) network-outbound remote:*:443 +// +// Configuration-based proxying asks the app to cooperate. This does not: +// a pf anchor rewrites the sandboxed process group's outbound 443 to this +// listener, so a direct dial IS the proxy. The app needs no proxy support, +// and cannot opt out — which is the enforcement guarantee the product sells. +// +// Relationship to the CONNECT forwarder in run_enforce_forwarder.go: that +// one serves clients that DO speak proxy and state their destination in a +// CONNECT line. This one serves clients that state nothing, and recovers the +// destination from the TLS SNI. Both then hand off to the same gateway with +// the same injected credentials. + +import ( + "errors" + "fmt" + "io" + "net" + "time" +) + +const ( + // How long to wait for a ClientHello before giving up. A redirected + // connection that never sends one cannot be routed. + transparentHelloTimeout = 10 * time.Second + // The port pf redirects. Only 443 is redirected: plain HTTP is already + // handled by the proxy env vars, and redirecting everything would catch + // non-TLS protocols whose destination we cannot recover. + transparentRedirectPort = 443 +) + +var errNoHelloTimeout = errors.New("timed out waiting for a TLS ClientHello") + +// serveTransparent runs the transparent listener until it is closed. +func serveTransparent(ln net.Listener, upstreamAddr, basicAuth string) { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go handleTransparentConn(conn, upstreamAddr, basicAuth) + } +} + +// handleTransparentConn recovers the destination from the SNI, opens a +// credentialed CONNECT tunnel to the gateway, replays the ClientHello, and +// pipes the rest. +// +// Fails CLOSED throughout: a connection whose destination cannot be +// determined is dropped, never guessed at and never sent direct. +func handleTransparentConn(conn net.Conn, upstreamAddr, basicAuth string) { + defer func() { _ = conn.Close() }() + + // Accept only traffic that pf actually redirected to us. + // + // The obvious check — "is the peer 127.0.0.1?" — is WRONG here, and + // measured to be so: pf's `route-to lo0` preserves the ORIGINAL source + // address, so a redirected connection arrives from the host's LAN IP + // (observed: 192.168.99.113) even though it traversed loopback. A + // loopback-only guard silently rejected every redirected connection + // while the redirect itself worked perfectly. + // + // What actually distinguishes redirected traffic is that our listener + // is bound to 127.0.0.1, so the kernel only ever delivers connections + // whose LOCAL address is loopback. Off-machine traffic cannot reach a + // loopback-bound socket at all. The meaningful check is therefore on + // the local side, and it is a genuine guard: it fails closed if the + // listener is ever bound to a wildcard or external address by mistake. + if la, ok := conn.LocalAddr().(*net.TCPAddr); !ok || !la.IP.IsLoopback() { + return + } + + hello, host, err := readClientHello(conn) + if err != nil { + return + } + + upstream, err := net.DialTimeout("tcp", upstreamAddr, enforceDialTimeout) + if err != nil { + return + } + defer func() { _ = upstream.Close() }() + + // host came from validateSNIHost, so it cannot contain CRLF and cannot + // forge headers on this connection. + target := net.JoinHostPort(host, fmt.Sprint(transparentRedirectPort)) + if _, err := fmt.Fprintf(upstream, + "CONNECT %s HTTP/1.1\r\nHost: %s\r\nProxy-Authorization: Basic %s\r\n\r\n", + target, target, basicAuth); err != nil { + return + } + if err := readConnectResponse(upstream); err != nil { + return + } + + // Replay the bytes consumed while sniffing, then pipe both ways. + if _, err := upstream.Write(hello); err != nil { + return + } + go func() { + _, _ = io.Copy(upstream, conn) + if tc, ok := upstream.(*net.TCPConn); ok { + _ = tc.CloseWrite() + } + }() + _, _ = io.Copy(conn, upstream) +} + +// readClientHello reads until a complete ClientHello is buffered, returning +// the raw bytes (for replay) and the parsed SNI host. +func readClientHello(conn net.Conn) ([]byte, string, error) { + if err := conn.SetReadDeadline(time.Now().Add(transparentHelloTimeout)); err != nil { + return nil, "", err + } + defer func() { _ = conn.SetReadDeadline(time.Time{}) }() + + buf := make([]byte, 0, 2048) + tmp := make([]byte, 2048) + for { + n, err := conn.Read(tmp) + if n > 0 { + buf = append(buf, tmp[:n]...) + host, perr := parseSNI(buf) + if perr == nil { + return buf, host, nil + } + if !errors.Is(perr, errHelloPartial) { + // Definitively not routable (not TLS, no SNI, malformed). + return nil, "", perr + } + if len(buf) > maxClientHelloSize { + return nil, "", errHelloTooBig + } + // else: partial, keep reading. + } + if err != nil { + if ne, ok := err.(net.Error); ok && ne.Timeout() { + return nil, "", errNoHelloTimeout + } + return nil, "", err + } + } +} + +// readConnectResponse consumes the gateway's CONNECT reply and verifies it +// succeeded. Without this the client's ClientHello would be written on top +// of an error response and the TLS handshake would fail with a confusing +// protocol error rather than a clean drop. +func readConnectResponse(upstream net.Conn) error { + if err := upstream.SetReadDeadline(time.Now().Add(enforceDialTimeout)); err != nil { + return err + } + defer func() { _ = upstream.SetReadDeadline(time.Time{}) }() + + // Read byte-wise to the end of the header block: anything buffered past + // it would belong to the tunnel and must not be swallowed. + var head []byte + one := make([]byte, 1) + for { + n, err := upstream.Read(one) + if err != nil { + return err + } + if n == 0 { + continue + } + head = append(head, one[0]) + if len(head) >= 4 && string(head[len(head)-4:]) == "\r\n\r\n" { + break + } + if len(head) > 8192 { + return fmt.Errorf("CONNECT response header exceeded 8KB") + } + } + if len(head) < 12 || string(head[:5]) != "HTTP/" { + return fmt.Errorf("malformed CONNECT response") + } + // "HTTP/1.1 200 ..." — status begins at offset 9. + if string(head[9:12]) != "200" { + return fmt.Errorf("gateway refused CONNECT: %s", string(head[9:12])) + } + return nil +} diff --git a/cmd/onecli/run_enforce_transparent_lifecycle_live_darwin_test.go b/cmd/onecli/run_enforce_transparent_lifecycle_live_darwin_test.go new file mode 100644 index 0000000..9adafb7 --- /dev/null +++ b/cmd/onecli/run_enforce_transparent_lifecycle_live_darwin_test.go @@ -0,0 +1,116 @@ +//go:build darwin + +package main + +// Lifecycle tests for the transparent session: the anchor must not outlive +// the run. +// +// This is a regression guard for a real leak. `onecli run` ends in +// syscall.Exec, which replaces the process image, so the parent's deferred +// Close and signal handlers never execute. The anchor persisted +// indefinitely after a run ended (measured: rdr=1 still present at t+10s), +// leaving the sandbox group redirected to a dead port and breaking the next +// enforced run in a confusing way. +// +// Cleanup therefore belongs to the detached sidecar, the only component +// that outlives the exec and can observe the agent exiting. + +import ( + "os" + "os/exec" + "strings" + "testing" + "time" +) + +func liveLifecycleEnabled(t *testing.T) { + t.Helper() + if os.Getenv("ONECLI_LIVE_CHAIN") != "1" { + t.Skip("set ONECLI_LIVE_CHAIN=1 to run anchor lifecycle tests") + } + if _, err := os.Stat(setgidHelperPath); err != nil { + t.Skipf("setgid helper not installed: %v", err) + } + if err := verifyTransparentSetup(); err != nil { + t.Skipf("transparent setup incomplete: %v", err) + } +} + +// anchorRuleCount reports how many rules our anchor currently holds. +func anchorRuleCount(t *testing.T) int { + t.Helper() + loaded, err := pfAnchorLoaded() + if err != nil { + return 0 + } + n := 0 + for _, l := range strings.Split(loaded, "\n") { + if strings.TrimSpace(l) != "" { + n++ + } + } + return n +} + +// TestAnchorDoesNotOutliveTheRun is the regression guard. +func TestAnchorDoesNotOutliveTheRun(t *testing.T) { + liveLifecycleEnabled(t) + + // Start clean so the assertion is unambiguous. + _ = pfFlushAnchor() + if n := anchorRuleCount(t); n != 0 { + t.Fatalf("anchor is not clean before the test: %d rules", n) + } + + // A session models what a run installs. + sess, err := startTransparentSession("http://x:aoc_test@127.0.0.1:10255/") + if err != nil { + t.Fatalf("starting session: %v", err) + } + if n := anchorRuleCount(t); n == 0 { + _ = sess.Close() + t.Fatal("the session loaded no rules; it is not governing anything") + } + + // Close covers the early-failure path (before exec). The normal path is + // the sidecar, exercised by TestSidecarFlushesAnchorOnParentExit. + if err := sess.Close(); err != nil { + t.Fatalf("closing session: %v", err) + } + if n := anchorRuleCount(t); n != 0 { + t.Fatalf("anchor still holds %d rules after Close; the sandbox group "+ + "stays redirected to a dead port", n) + } +} + +// TestSidecarFlushesAnchorOnParentExit exercises the NORMAL teardown path +// end to end through the real binary: a run that starts, installs the +// anchor, and exits. Nothing may be left behind. +func TestSidecarFlushesAnchorOnParentExit(t *testing.T) { + liveLifecycleEnabled(t) + + bin := os.Getenv("ONECLI_TEST_BINARY") + if bin == "" { + t.Skip("set ONECLI_TEST_BINARY to the built onecli binary") + } + _ = pfFlushAnchor() + + cmd := exec.Command(bin, "run", "--enforce", "--", "bash", "-c", "true") + cmd.Env = append(os.Environ(), enforceTransparentEnv+"=1") + if out, err := cmd.CombinedOutput(); err != nil { + t.Skipf("enforced run did not start (env-dependent): %v\n%s", err, out) + } + + // The sidecar polls its parent every enforceParentPollTime, so allow a + // few intervals before declaring a leak. + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + if anchorRuleCount(t) == 0 { + return // cleaned up + } + time.Sleep(500 * time.Millisecond) + } + t.Fatalf("the anchor still holds %d rules after the run exited; "+ + "group %s stays redirected with no listener", + anchorRuleCount(t), transparentGroupName) +} diff --git a/cmd/onecli/run_enforce_transparent_live_darwin_test.go b/cmd/onecli/run_enforce_transparent_live_darwin_test.go new file mode 100644 index 0000000..dd735bd --- /dev/null +++ b/cmd/onecli/run_enforce_transparent_live_darwin_test.go @@ -0,0 +1,197 @@ +//go:build darwin + +package main + +// Live end-to-end proof for transparent redirect. +// +// Gated behind ONECLI_LIVE_TRANSPARENT=1 because it makes real network +// connections. What it proves cannot be proven by a unit test: that a real +// TLS client, given NO proxy configuration whatsoever, completes a real +// handshake with a real remote host purely because its connection landed on +// the transparent listener. +// +// This is the exact shape of the Cursor extension-host case. The listener +// stands in for what pf produces; pf's own contribution (getting the packet +// here) is verified separately by the rule tests and the anchor check. + +import ( + "context" + "crypto/tls" + "encoding/base64" + "io" + "net" + "net/http" + "os" + "strings" + "testing" + "time" +) + +func liveTransparentEnabled(t *testing.T) { + t.Helper() + if os.Getenv("ONECLI_LIVE_TRANSPARENT") != "1" { + t.Skip("set ONECLI_LIVE_TRANSPARENT=1 to run live transparent-redirect tests") + } +} + +// directCONNECTGateway is a minimal real proxy: it honors CONNECT by dialing +// the true destination. It stands in for the OneCLI gateway so this test +// exercises the listener against genuine remote endpoints without depending +// on gateway availability or credentials. +type directCONNECTGateway struct { + ln net.Listener + sawHosts chan string +} + +func newDirectGateway(t *testing.T) *directCONNECTGateway { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("gateway listen: %v", err) + } + g := &directCONNECTGateway{ln: ln, sawHosts: make(chan string, 16)} + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + go g.handle(c) + } + }() + t.Cleanup(func() { _ = ln.Close() }) + return g +} + +func (g *directCONNECTGateway) handle(conn net.Conn) { + defer func() { _ = conn.Close() }() + + var head []byte + one := make([]byte, 1) + for { + n, err := conn.Read(one) + if err != nil || n == 0 { + return + } + head = append(head, one[0]) + if len(head) >= 4 && string(head[len(head)-4:]) == "\r\n\r\n" { + break + } + if len(head) > 8192 { + return + } + } + line := strings.Split(string(head), "\r\n")[0] + parts := strings.Fields(line) + if len(parts) < 2 || parts[0] != "CONNECT" { + return + } + target := parts[1] + select { + case g.sawHosts <- target: + default: + } + + upstream, err := net.DialTimeout("tcp", target, 10*time.Second) + if err != nil { + _, _ = conn.Write([]byte("HTTP/1.1 502 Bad Gateway\r\n\r\n")) + return + } + defer func() { _ = upstream.Close() }() + if _, err := conn.Write([]byte("HTTP/1.1 200 Connection Established\r\n\r\n")); err != nil { + return + } + go func() { _, _ = io.Copy(upstream, conn) }() + _, _ = io.Copy(conn, upstream) +} + +// TestLiveTransparentCompletesRealTLS is the proof. +// +// A client with no proxy config dials the transparent listener and completes +// a genuine TLS handshake with a real host, verifying the real certificate. +// A working handshake is unforgeable evidence the ClientHello was replayed +// intact and bytes flow correctly in both directions. +func TestLiveTransparentCompletesRealTLS(t *testing.T) { + liveTransparentEnabled(t) + + gw := newDirectGateway(t) + creds := base64.StdEncoding.EncodeToString([]byte("x:test")) + addr := startTransparent(t, gw.ln.Addr().String(), creds) + + for _, host := range []string{ + "api.anthropic.com", + "agentn.global.api5.cursor.sh", // the host that failed under enforce + } { + t.Run(host, func(t *testing.T) { + raw, err := net.DialTimeout("tcp", addr, 10*time.Second) + if err != nil { + t.Fatalf("dial listener: %v", err) + } + defer func() { _ = raw.Close() }() + + // Full verification against the real chain. If the listener + // mangled the handshake this fails. + tlsConn := tls.Client(raw, &tls.Config{ServerName: host}) + _ = tlsConn.SetDeadline(time.Now().Add(20 * time.Second)) + if err := tlsConn.Handshake(); err != nil { + t.Fatalf("TLS handshake through the transparent listener failed: %v", err) + } + cs := tlsConn.ConnectionState() + if len(cs.PeerCertificates) == 0 { + t.Fatal("no peer certificate") + } + if err := tlsConn.VerifyHostname(host); err != nil { + t.Fatalf("certificate does not match %s: %v", host, err) + } + t.Logf("handshake ok: %s, tls=%#x, cn=%q", + host, cs.Version, cs.PeerCertificates[0].Subject.CommonName) + }) + } + + // The gateway must have seen a CONNECT for each host, proving the + // destination was recovered from the SNI rather than guessed. + close(gw.sawHosts) + var seen []string + for h := range gw.sawHosts { + seen = append(seen, h) + } + if len(seen) < 2 { + t.Fatalf("gateway saw %v; expected a CONNECT per host", seen) + } +} + +// TestLiveTransparentServesRealHTTP proves the tunnel carries application +// traffic, not just a handshake: a full HTTPS request/response round trip +// through a client that was never told a proxy exists. +func TestLiveTransparentServesRealHTTP(t *testing.T) { + liveTransparentEnabled(t) + + gw := newDirectGateway(t) + creds := base64.StdEncoding.EncodeToString([]byte("x:test")) + addr := startTransparent(t, gw.ln.Addr().String(), creds) + + // A transport whose DialContext always lands on the listener: exactly + // what pf does to a process that dials directly. Note there is no + // Proxy field set — the client has no proxy configuration at all, and + // still ends up governed. + tr := &http.Transport{Proxy: nil} + tr.DialContext = func(ctx context.Context, _, _ string) (net.Conn, error) { + var d net.Dialer + return d.DialContext(ctx, "tcp", addr) + } + c := &http.Client{Timeout: 30 * time.Second, Transport: tr} + + resp, err := c.Get("https://api.anthropic.com/v1/messages") + if err != nil { + t.Fatalf("HTTPS request through the transparent listener failed: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + // Any HTTP status proves the round trip; the endpoint rejects an + // unauthenticated POST-shaped GET, which is fine. + if resp.StatusCode == 0 { + t.Fatal("no HTTP response") + } + t.Logf("round trip ok: HTTP %d from api.anthropic.com with NO proxy config", + resp.StatusCode) +} diff --git a/cmd/onecli/run_enforce_transparent_session_darwin.go b/cmd/onecli/run_enforce_transparent_session_darwin.go new file mode 100644 index 0000000..73edce1 --- /dev/null +++ b/cmd/onecli/run_enforce_transparent_session_darwin.go @@ -0,0 +1,138 @@ +//go:build darwin + +package main + +// Session lifecycle for transparent redirect: verify preconditions, spawn +// the listener, load the anchor, and guarantee it is removed on exit. +// +// The invariant this file exists to hold: the widened Seatbelt profile and +// the loaded pf anchor must be installed together and removed together. +// Transparent mode permits outbound 443 at the OS layer, and only the +// anchor's default-deny governs it. A widened profile with no anchor is +// ungoverned direct egress, so every path here either achieves BOTH or +// fails and cleans up. + +import ( + "fmt" + "os" + "os/signal" + "strings" + "syscall" +) + +// transparentSession owns the resources an enforced run installs. +type transparentSession struct { + Port uint16 + GID int + loaded bool +} + +// startTransparentSession verifies setup, binds the listener, and loads the +// anchor. Returns a session whose Close() restores the machine. +// +// Fails closed at every step: any error tears down what was already done, so +// a partial failure never leaves a widened profile with no anchor. +func startTransparentSession(gatewayProxyURL string) (*transparentSession, error) { + // pf is reference counted on macOS and can be off even after setup + // enabled it, so bring it up before verifying rather than failing on a + // condition we can fix. + if err := pfEnsureEnabled(); err != nil { + return nil, err + } + if err := verifyTransparentSetup(); err != nil { + return nil, err + } + gid, err := sandboxGID(transparentGroupName) + if err != nil { + return nil, err + } + if _, _, err := parseGatewayProxy(gatewayProxyURL); err != nil { + return nil, err + } + + // A DETACHED sidecar, not an in-process goroutine: `onecli run` ends in + // syscall.Exec, which replaces this process image with the agent. An + // in-process listener would die there while the anchor survived, and + // every redirected connection would hit a dead port. Observed exactly + // that way before this changed (gid=700 adopted, curl RST in 2ms). + port, err := spawnTransparentListener(gatewayProxyURL) + if err != nil { + return nil, err + } + + s := &transparentSession{Port: port, GID: gid} + + rules, err := pfRules(port, gid) + if err != nil { + return nil, err + } + if err := pfLoadAnchor(rules); err != nil { + return nil, err + } + s.loaded = true + + // Verify the anchor is REALLY loaded rather than trusting the exit + // code. pfctl can succeed while rules end up inert, and this codebase + // has been bitten by exactly that shape before. + if err := s.verifyLoaded(); err != nil { + _ = s.Close() + return nil, err + } + + // The anchor outlives a crashed process unless we remove it. Signal + // handling is not politeness here: a stale anchor would keep + // redirecting a group whose listener is gone, breaking all egress for + // it until someone flushed it by hand. + s.installCleanupHandlers() + return s, nil +} + +// verifyLoaded confirms our rules are present in the anchor. +func (s *transparentSession) verifyLoaded() error { + loaded, err := pfAnchorLoaded() + if err != nil { + return err + } + if loaded == "" { + return fmt.Errorf("the pf anchor is empty after loading; rules would not apply") + } + // The redirect target port is the one thing that must match exactly: + // a stale anchor from a previous session would point at a dead port. + want := fmt.Sprintf("port %d", s.Port) + if !strings.Contains(loaded, want) { + return fmt.Errorf("the loaded anchor does not target this session's "+ + "listener (%s); a stale anchor may be present", want) + } + return nil +} + +// installCleanupHandlers removes the anchor on SIGINT/SIGTERM. +func (s *transparentSession) installCleanupHandlers() { + ch := make(chan os.Signal, 1) + signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) + go func() { + <-ch + _ = s.Close() + os.Exit(130) + }() +} + +// Close flushes the anchor, for the paths where this process still exists +// to do it: a failure during setup, or a run that returns before exec. +// +// It is NOT the normal teardown path. `onecli run` ends in syscall.Exec, +// which replaces this process image, so neither defers nor signal handlers +// installed here survive. The detached sidecar owns cleanup for the normal +// case, and this covers the early-failure cases it cannot see. +// +// Safe to call twice. +func (s *transparentSession) Close() error { + var firstErr error + if s.loaded { + if err := pfFlushAnchor(); err != nil { + firstErr = err + } + s.loaded = false + } + return firstErr +} diff --git a/cmd/onecli/run_enforce_transparent_session_darwin_test.go b/cmd/onecli/run_enforce_transparent_session_darwin_test.go new file mode 100644 index 0000000..a764a33 --- /dev/null +++ b/cmd/onecli/run_enforce_transparent_session_darwin_test.go @@ -0,0 +1,126 @@ +//go:build darwin + +package main + +// Tests for the transparent session lifecycle. +// +// The property under test is the one that keeps the design honest: the +// widened Seatbelt profile and the loaded pf anchor must exist together or +// not at all. These run without root by exercising the failure paths, which +// is where the security-relevant behavior lives. + +import ( + "strings" + "testing" +) + +// TestTransparentSessionRefusesWithoutSetup is the guard that matters most. +// Transparent mode permits outbound 443 in the sandbox profile; if a session +// could start without a verified anchor, that permission would be +// ungoverned direct egress. +// +// On a machine without setup, startTransparentSession MUST fail. +func TestTransparentSessionRefusesWithoutSetup(t *testing.T) { + if err := verifyTransparentSetup(); err == nil { + t.Skip("this machine IS set up; the refusal path cannot be tested here") + } + _, err := startTransparentSession("http://x:aoc_test@127.0.0.1:1/") + if err == nil { + t.Fatal("a transparent session started without verified setup; the " + + "widened profile would be ungoverned") + } + t.Logf("correctly refused: %v", err) +} + +// TestVerifyTransparentSetupNamesTheFailure: a generic "not ready" would +// leave an operator guessing between four different causes, two of which +// (pf disabled, anchor unreferenced) fail silently at the pf layer. +func TestVerifyTransparentSetupNamesTheFailure(t *testing.T) { + err := verifyTransparentSetup() + if err == nil { + t.Skip("this machine is fully set up") + } + msg := err.Error() + known := []string{ + transparentGroupName, // group missing + "pf is disabled", + "does not reference", + "pfctl", + } + for _, k := range known { + if strings.Contains(msg, k) { + t.Logf("failure names its cause: %v", err) + return + } + } + t.Fatalf("error does not identify which precondition failed: %v", err) +} + +// TestTransparentSetupScriptIsSafe reviews the generated script for the +// properties that make it trustworthy. It edits sudoers, so its content is +// a security decision, not a formatting one. +func TestTransparentSetupScriptIsSafe(t *testing.T) { + script := transparentSetupScript(700, "testuser") + + // Scoped privilege only: pfctl and nothing else. + if !strings.Contains(script, "NOPASSWD: "+pfctlPath) { + t.Fatal("script does not grant pfctl access") + } + for _, forbidden := range []string{ + "NOPASSWD: ALL", + "ALL=(ALL)", + "(ALL:ALL)", + } { + if strings.Contains(script, forbidden) { + t.Fatalf("script grants blanket root via %q", forbidden) + } + } + + // Must validate its own sudoers edit: a malformed sudoers file can + // lock the user out of sudo entirely. + if !strings.Contains(script, "visudo -cf") { + t.Fatal("script does not validate the sudoers file it writes") + } + + // Idempotent: guarded group creation. + if !strings.Contains(script, "if ! dscl") { + t.Fatal("script is not idempotent for group creation") + } + + // Must tell the user how to undo it. + if !strings.Contains(script, "To undo") { + t.Fatal("script does not print an undo path") + } + + // Must not use a system GID. + if strings.Contains(script, "PrimaryGroupID 0") { + t.Fatal("script would create a group with GID 0") + } +} + +// TestTransparentSetupScriptRejectsSystemGID guards the GID floor. +func TestNextFreeGIDIsAboveSystemRange(t *testing.T) { + gid, err := nextFreeGID() + if err != nil { + t.Skipf("cannot enumerate groups: %v", err) + } + if gid < transparentGIDFloor { + t.Fatalf("nextFreeGID returned %d, below the system floor %d", + gid, transparentGIDFloor) + } + if err := validateSandboxGID(gid); err != nil { + t.Fatalf("nextFreeGID returned a GID pf cannot use: %v", err) + } +} + +// TestTransparentSessionCloseIsIdempotent: Close runs from a signal handler +// AND from the normal path, so a double call must not error or panic. +func TestTransparentSessionCloseIsIdempotent(t *testing.T) { + s := &transparentSession{} // nothing loaded, nothing bound + if err := s.Close(); err != nil { + t.Fatalf("first Close: %v", err) + } + if err := s.Close(); err != nil { + t.Fatalf("second Close: %v", err) + } +} diff --git a/cmd/onecli/run_enforce_transparent_sidecar_darwin.go b/cmd/onecli/run_enforce_transparent_sidecar_darwin.go new file mode 100644 index 0000000..a3000f2 --- /dev/null +++ b/cmd/onecli/run_enforce_transparent_sidecar_darwin.go @@ -0,0 +1,151 @@ +//go:build darwin + +package main + +// Detached sidecar for the transparent listener. +// +// Why this exists: `onecli run` ends in syscall.Exec, which REPLACES the +// process image with the agent. An in-process listener goroutine dies at +// that moment while the pf anchor survives, so every redirected connection +// hits a dead port. Observed exactly that way: gid=700 proved the group was +// adopted and curl failed in 2ms, the signature of a loopback RST. +// +// The CONNECT forwarder already solved this by forking a detached child that +// inherits the bound listener as FD 3. This mirrors that pattern rather than +// inventing a second lifecycle model. + +import ( + "fmt" + "net" + "os" + "os/exec" + "os/signal" + "strconv" + "syscall" + "time" +) + +const ( + transparentSidecarFlag = "__enforce-transparent" + // Carries the gateway proxy URL (with credentials) to the sidecar via + // env, not argv: argv is world-readable in `ps`. + transparentProxyURLEnv = "ONECLI_TRANSPARENT_PROXY_URL" +) + +// spawnTransparentListener binds the listener, forks a detached sidecar +// holding it, and returns the port. Must be called BEFORE syscall.Exec. +func spawnTransparentListener(gatewayProxyURL string) (uint16, error) { + if _, _, err := parseGatewayProxy(gatewayProxyURL); err != nil { + return 0, err + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return 0, fmt.Errorf("binding transparent listener: %w", err) + } + tcpLn, ok := ln.(*net.TCPListener) + if !ok { + _ = ln.Close() + return 0, fmt.Errorf("unexpected listener type %T", ln) + } + port := uint16(tcpLn.Addr().(*net.TCPAddr).Port) + + lnFile, err := tcpLn.File() + if err != nil { + _ = ln.Close() + return 0, fmt.Errorf("duplicating listener fd: %w", err) + } + defer func() { + _ = lnFile.Close() + _ = ln.Close() + }() + + self, err := os.Executable() + if err != nil { + return 0, fmt.Errorf("resolving own binary: %w", err) + } + cmd := exec.Command(self, transparentSidecarFlag, + "--parent-pid", strconv.Itoa(os.Getpid())) + cmd.Env = append(os.Environ(), transparentProxyURLEnv+"="+gatewayProxyURL) + cmd.ExtraFiles = []*os.File{lnFile} // FD 3 in the child + cmd.SysProcAttr = detachedSysProcAttr() + if err := cmd.Start(); err != nil { + return 0, fmt.Errorf("starting transparent listener: %w", err) + } + _ = cmd.Process.Release() + return port, nil +} + +// parseTransparentSidecarArgs recognizes the hidden sidecar invocation. +func parseTransparentSidecarArgs(argv []string) (parentPID int, ok bool) { + if len(argv) < 1 || argv[0] != transparentSidecarFlag { + return 0, false + } + for i := 1; i < len(argv)-1; i++ { + if argv[i] == "--parent-pid" { + pid, err := strconv.Atoi(argv[i+1]) + if err != nil { + return 0, false + } + return pid, true + } + } + return 0, false +} + +// runTransparentSidecar serves the inherited listener until the agent exits. +func runTransparentSidecar(parentPID int) { + proxyURL := os.Getenv(transparentProxyURLEnv) + upstreamAddr, basicAuth, err := parseGatewayProxy(proxyURL) + if err != nil { + return + } + lnFile := os.NewFile(3, "onecli-transparent-listener") + if lnFile == nil { + return + } + ln, err := net.FileListener(lnFile) + if err != nil { + return + } + defer func() { _ = ln.Close() }() + + // The sidecar owns anchor cleanup, because nothing else can. + // + // `onecli run` ends in syscall.Exec, which REPLACES the process image + // with the agent, so the parent's deferred Close and signal handlers + // never run. Measured: after an enforced run ended, the sidecar exited + // correctly but the anchor persisted indefinitely (rdr=1 at t+10s), + // leaving group 700 redirected to a dead port. That would break the + // NEXT enforced run in a confusing way. + // + // The sidecar is the only component that outlives the exec and knows + // when the agent is gone, so it flushes the anchor before exiting. + cleanup := func(code int) { + if err := pfFlushAnchor(); err != nil { + // Nothing useful to do with the error here: stderr is + // detached. Exiting without flushing is the worse outcome, so + // the attempt is unconditional and best-effort. + _ = err + } + os.Exit(code) + } + + go func() { + for { + time.Sleep(enforceParentPollTime) + if !enforceProcessAlive(parentPID) { + cleanup(0) + } + } + }() + + // Signals too: a killed sidecar must not leave the anchor behind. + sigCh := make(chan os.Signal, 1) + signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) + go func() { + <-sigCh + cleanup(130) + }() + + serveTransparent(ln, upstreamAddr, basicAuth) +} diff --git a/cmd/onecli/run_enforce_transparent_test.go b/cmd/onecli/run_enforce_transparent_test.go new file mode 100644 index 0000000..9ca66cc --- /dev/null +++ b/cmd/onecli/run_enforce_transparent_test.go @@ -0,0 +1,330 @@ +package main + +// End-to-end tests for the transparent listener. +// +// The scenario reproduced here is exactly the Cursor extension-host failure: +// a client that speaks NO proxy protocol, dials what it believes is a remote +// host, and states its destination only through the TLS SNI. These tests do +// not require pf or root — the redirect is simulated by dialing the listener +// directly, which is precisely what pf produces. + +import ( + "crypto/tls" + "encoding/base64" + "fmt" + "net" + "strings" + "sync" + "testing" + "time" +) + +// fakeGateway accepts CONNECT, records the request line and credentials, and +// echoes tunnel bytes back so the client can verify the pipe. +type fakeGateway struct { + ln net.Listener + mu sync.Mutex + requests []string + authz []string + refuse bool +} + +func newFakeGateway(t *testing.T, refuse bool) *fakeGateway { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("gateway listen: %v", err) + } + g := &fakeGateway{ln: ln, refuse: refuse} + go g.serve() + t.Cleanup(func() { _ = ln.Close() }) + return g +} + +func (g *fakeGateway) addr() string { return g.ln.Addr().String() } + +func (g *fakeGateway) serve() { + for { + conn, err := g.ln.Accept() + if err != nil { + return + } + go g.handle(conn) + } +} + +func (g *fakeGateway) handle(conn net.Conn) { + defer func() { _ = conn.Close() }() + + var head []byte + one := make([]byte, 1) + for { + n, err := conn.Read(one) + if err != nil || n == 0 { + return + } + head = append(head, one[0]) + if len(head) >= 4 && string(head[len(head)-4:]) == "\r\n\r\n" { + break + } + if len(head) > 8192 { + return + } + } + lines := strings.Split(string(head), "\r\n") + + g.mu.Lock() + g.requests = append(g.requests, lines[0]) + for _, l := range lines { + if strings.HasPrefix(strings.ToLower(l), "proxy-authorization:") { + g.authz = append(g.authz, strings.TrimSpace(l[len("proxy-authorization:"):])) + } + } + g.mu.Unlock() + + if g.refuse { + _, _ = conn.Write([]byte("HTTP/1.1 407 Proxy Authentication Required\r\nContent-Length: 0\r\n\r\n")) + return + } + if _, err := conn.Write([]byte("HTTP/1.1 200 Connection Established\r\n\r\n")); err != nil { + return + } + // Echo the tunnel so the caller can assert bytes flow through. + buf := make([]byte, 4096) + for { + n, err := conn.Read(buf) + if n > 0 { + if _, werr := conn.Write(buf[:n]); werr != nil { + return + } + } + if err != nil { + return + } + } +} + +func (g *fakeGateway) snapshot() ([]string, []string) { + g.mu.Lock() + defer g.mu.Unlock() + return append([]string(nil), g.requests...), append([]string(nil), g.authz...) +} + +// startTransparent runs the listener under test against a gateway. +func startTransparent(t *testing.T, gwAddr, basicAuth string) string { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("transparent listen: %v", err) + } + go serveTransparent(ln, gwAddr, basicAuth) + t.Cleanup(func() { _ = ln.Close() }) + return ln.Addr().String() +} + +// TestTransparentTunnelsBySNI is the core proof: a client with NO proxy +// configuration dials the listener, and its intended destination — carried +// only in the SNI — reaches the gateway as a credentialed CONNECT. +func TestTransparentTunnelsBySNI(t *testing.T) { + const wantHost = "agentn.global.api5.cursor.sh" + creds := base64.StdEncoding.EncodeToString([]byte("x:aoc_test_token")) + + gw := newFakeGateway(t, false) + addr := startTransparent(t, gw.addr(), creds) + + // A real TLS client. It has no idea it is being proxied: it simply + // dials and announces its ServerName, exactly like Cursor's ext host. + conn, err := net.DialTimeout("tcp", addr, 5*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer func() { _ = conn.Close() }() + + tlsConn := tls.Client(conn, &tls.Config{ServerName: wantHost}) + _ = tlsConn.SetDeadline(time.Now().Add(3 * time.Second)) + // Handshake fails: our fake gateway echoes rather than serving TLS. + // Irrelevant here — what matters is what reached the gateway. + _ = tlsConn.Handshake() + + deadline := time.Now().Add(3 * time.Second) + var reqs, auths []string + for time.Now().Before(deadline) { + reqs, auths = gw.snapshot() + if len(reqs) > 0 { + break + } + time.Sleep(20 * time.Millisecond) + } + + if len(reqs) == 0 { + t.Fatal("gateway saw no CONNECT: the transparent listener did not tunnel") + } + wantLine := fmt.Sprintf("CONNECT %s:443 HTTP/1.1", wantHost) + if reqs[0] != wantLine { + t.Fatalf("gateway got %q, want %q", reqs[0], wantLine) + } + if len(auths) == 0 || auths[0] != "Basic "+creds { + t.Fatalf("credentials not injected: got %v", auths) + } +} + +// TestTransparentReplaysClientHello proves the sniffed bytes are forwarded, +// not swallowed. Dropping them would break every real TLS handshake. +func TestTransparentReplaysClientHello(t *testing.T) { + gw := newFakeGateway(t, false) + addr := startTransparent(t, gw.addr(), "dGVzdA==") + + conn, err := net.DialTimeout("tcp", addr, 5*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer func() { _ = conn.Close() }() + + hello := captureClientHello(t, "api.anthropic.com") + if _, err := conn.Write(hello); err != nil { + t.Fatalf("write hello: %v", err) + } + + // The gateway echoes, so the replayed hello must come back verbatim. + _ = conn.SetReadDeadline(time.Now().Add(3 * time.Second)) + got := make([]byte, len(hello)) + n, err := ioReadFull(conn, got) + if err != nil { + t.Fatalf("reading echo after %d bytes: %v", n, err) + } + if string(got) != string(hello) { + t.Fatal("the ClientHello was not replayed byte-for-byte to the gateway") + } +} + +// TestTransparentDropsNonTLS: a redirected connection whose destination +// cannot be recovered must be dropped, never guessed and never sent direct. +func TestTransparentDropsNonTLS(t *testing.T) { + gw := newFakeGateway(t, false) + addr := startTransparent(t, gw.addr(), "dGVzdA==") + + conn, err := net.DialTimeout("tcp", addr, 5*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer func() { _ = conn.Close() }() + + if _, err := conn.Write([]byte("GET / HTTP/1.1\r\nHost: evil.com\r\n\r\n")); err != nil { + t.Fatalf("write: %v", err) + } + + _ = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + buf := make([]byte, 64) + if _, err := conn.Read(buf); err == nil { + t.Fatal("non-TLS traffic got a response; it must be dropped") + } + if reqs, _ := gw.snapshot(); len(reqs) != 0 { + t.Fatalf("non-TLS traffic reached the gateway: %v", reqs) + } +} + +// TestTransparentDropsOnGatewayRefusal: if the gateway rejects the CONNECT, +// the client must get a closed connection rather than a broken tunnel that +// looks like a TLS error. +func TestTransparentDropsOnGatewayRefusal(t *testing.T) { + gw := newFakeGateway(t, true) // refuse + addr := startTransparent(t, gw.addr(), "dGVzdA==") + + conn, err := net.DialTimeout("tcp", addr, 5*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer func() { _ = conn.Close() }() + + hello := captureClientHello(t, "api.anthropic.com") + if _, err := conn.Write(hello); err != nil { + t.Fatalf("write hello: %v", err) + } + _ = conn.SetReadDeadline(time.Now().Add(3 * time.Second)) + buf := make([]byte, 64) + if n, err := conn.Read(buf); err == nil && n > 0 { + t.Fatalf("got %d bytes after a refused CONNECT; want a closed conn", n) + } +} + +// TestTransparentRejectsSNIHeaderInjection: an SNI carrying CRLF must never +// reach the gateway, or a sandboxed process could forge its own +// Proxy-Authorization on the tunnel. +func TestTransparentRejectsSNIHeaderInjection(t *testing.T) { + gw := newFakeGateway(t, false) + addr := startTransparent(t, gw.addr(), "dGVzdA==") + + // Hand-built hello with a malicious SNI; crypto/tls will not emit one. + evil := buildClientHelloWithSNI("evil.com\r\nProxy-Authorization: Basic AAAA") + + conn, err := net.DialTimeout("tcp", addr, 5*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer func() { _ = conn.Close() }() + if _, err := conn.Write(evil); err != nil { + t.Fatalf("write: %v", err) + } + + time.Sleep(500 * time.Millisecond) + reqs, auths := gw.snapshot() + if len(reqs) != 0 { + t.Fatalf("an injected SNI reached the gateway: %v / %v", reqs, auths) + } +} + +// buildClientHelloWithSNI constructs a minimal but structurally valid +// ClientHello carrying an arbitrary server_name, including values that a +// conforming TLS stack would never produce. +func buildClientHelloWithSNI(host string) []byte { + name := []byte(host) + + var sni []byte + sni = append(sni, 0x00) // name_type + sni = append(sni, byte(len(name)>>8), byte(len(name))) // length + sni = append(sni, name...) + var sniList []byte + sniList = append(sniList, byte(len(sni)>>8), byte(len(sni))) + sniList = append(sniList, sni...) + + var ext []byte + ext = append(ext, 0x00, 0x00) // server_name + ext = append(ext, byte(len(sniList)>>8), byte(len(sniList))) + ext = append(ext, sniList...) + + var exts []byte + exts = append(exts, byte(len(ext)>>8), byte(len(ext))) + exts = append(exts, ext...) + + var body []byte + body = append(body, 0x03, 0x03) // client_version + body = append(body, make([]byte, 32)...) // random + body = append(body, 0x00) // session_id (empty) + body = append(body, 0x00, 0x02, 0x13, 0x01) // cipher_suites + body = append(body, 0x01, 0x00) // compression_methods + body = append(body, exts...) + + var hs []byte + hs = append(hs, 0x01) // ClientHello + hs = append(hs, byte(len(body)>>16), byte(len(body)>>8), byte(len(body))) + hs = append(hs, body...) + + var rec []byte + rec = append(rec, 0x16, 0x03, 0x01) + rec = append(rec, byte(len(hs)>>8), byte(len(hs))) + rec = append(rec, hs...) + return rec +} + +// ioReadFull is io.ReadFull, spelled locally to keep the import list tight. +func ioReadFull(c net.Conn, buf []byte) (int, error) { + total := 0 + for total < len(buf) { + n, err := c.Read(buf[total:]) + total += n + if err != nil { + return total, err + } + } + return total, nil +} diff --git a/cmd/onecli/run_enforce_wrap.go b/cmd/onecli/run_enforce_wrap.go index b12af98..6f219e7 100644 --- a/cmd/onecli/run_enforce_wrap.go +++ b/cmd/onecli/run_enforce_wrap.go @@ -17,6 +17,7 @@ package main // docker.sock rule stayed broken while a text assertion kept passing. import ( + "fmt" "net" "net/url" "strconv" @@ -50,8 +51,11 @@ func rewriteProxyEnvToLoopback(env map[string]string, port uint16) { // enforceWrapQuirkArgs returns extra argv appended for agents whose own // runtime needs adjusting to live inside the wrap. Appended last so it // wins over defaults (and is visible in ps for auditability). -func enforceWrapQuirkArgs(framework string) []string { - if framework == "codex" { +// forwarderPort is the loopback port all sandboxed egress must exit +// through; Chromium-based agents need it as a native flag (see below). +func enforceWrapQuirkArgs(framework string, forwarderPort uint16) []string { + switch framework { + case "codex": // Codex's internal Seatbelt (sandbox_apply) gets EPERM inside an // outer Seatbelt profile, which breaks every shell command it // runs. Disable its inner sandbox: network governance moves to @@ -59,6 +63,29 @@ func enforceWrapQuirkArgs(framework string) []string { // filesystem confinement falls back to Codex's approval flow // plus the deferred-egress denies in the profile. return []string{"-c", "sandbox_mode=danger-full-access"} + case "cursor", "agent": + args := []string{ + // Electron/Chromium spawns renderer, GPU and network child + // processes that each try to create their OWN sandbox. Inside + // our outer Seatbelt profile that call is denied ("sandbox + // initialization failed: Operation not permitted"), the GPU + // process dies repeatedly and Chromium aborts the app ("GPU + // process isn't usable. Goodbye."). Disabling Chromium's inner + // sandbox lets the editor run; egress stays confined by the + // outer profile, which is the guarantee we actually make. + // Same shape as the Codex quirk: one sandbox, ours, not two. + "--no-sandbox", + } + if forwarderPort != 0 { + // The editor's own network stack (Electron's SimpleURLLoader) + // does NOT reliably honor VS Code's `http.proxy` setting — it + // failed with net::ERR_INVALID_ARGUMENT while the identical + // request succeeded through the same forwarder via curl. Since + // we launch the binary ourselves, configure Chromium natively + // instead of hoping the app-level setting is respected. + args = append(args, fmt.Sprintf("--proxy-server=http://127.0.0.1:%d", forwarderPort)) + } + return args } return nil } @@ -66,16 +93,19 @@ func enforceWrapQuirkArgs(framework string) []string { // enforceWrapNotice returns the agent-specific stderr notice for quirks // that change the agent's own behavior, or "". func enforceWrapNotice(framework string) string { - if framework == "codex" { + switch framework { + case "codex": return "onecli: Codex's internal sandbox is disabled under enforce — the OneCLI sandbox governs all egress instead; filesystem safety falls back to Codex approvals." + case "cursor", "agent": + return "onecli: Chromium's internal sandbox is disabled under enforce (it cannot nest inside ours) — the OneCLI sandbox governs all editor egress instead." } return "" } // enforceWrapArgv builds the argv that runs the agent confined: the // launcher's argv with per-agent quirk flags appended last. -func enforceWrapArgv(profilePath, binary string, agentArgs []string, framework string) []string { - args := append(append([]string{}, agentArgs...), enforceWrapQuirkArgs(framework)...) +func enforceWrapArgv(profilePath, binary string, agentArgs []string, framework string, forwarderPort uint16) []string { + args := append(append([]string{}, agentArgs...), enforceWrapQuirkArgs(framework, forwarderPort)...) return sandbox.WrapArgv(profilePath, binary, args) } diff --git a/cmd/onecli/run_enforce_wrap_bypass_live_test.go b/cmd/onecli/run_enforce_wrap_bypass_live_test.go index 4f7f0d7..51c0ad4 100644 --- a/cmd/onecli/run_enforce_wrap_bypass_live_test.go +++ b/cmd/onecli/run_enforce_wrap_bypass_live_test.go @@ -17,6 +17,7 @@ package main import ( "os" + "path/filepath" "strings" "testing" @@ -30,9 +31,17 @@ func TestLiveEnforceWrapBypasses(t *testing.T) { // Port 1: nothing listens there, so every probe destination is outside // the allow. The forwarder-reachable probe is skipped by sandboxProbes() // when no real port is supplied. - profile, err := sandbox.Materialize(1) - if err != nil { - t.Fatalf("materializing profile: %v", err) + // + // Rendered to a TEMP path, never via sandbox.Materialize: that writes the + // shared ~/.onecli/enforce-wrap.sb, which is the exact file a live + // enforced session is running under. Running `go test` would then rewrite + // a real user's sandbox policy to allow only port 1 — silently breaking + // their session's egress and, worse, invalidating any measurement taken + // against it afterwards. That happened during development and cost hours + // of misdiagnosis, so the test now owns its own file. + profile := filepath.Join(t.TempDir(), "enforce-wrap.sb") + if err := os.WriteFile(profile, []byte(sandbox.Profile(1)), 0o600); err != nil { + t.Fatalf("writing probe profile: %v", err) } for _, p := range sandboxProbes() { diff --git a/cmd/onecli/run_enforce_wrap_mode_darwin.go b/cmd/onecli/run_enforce_wrap_mode_darwin.go new file mode 100644 index 0000000..d4cd3d7 --- /dev/null +++ b/cmd/onecli/run_enforce_wrap_mode_darwin.go @@ -0,0 +1,21 @@ +//go:build darwin + +package main + +// resolveEnforceWrapMode picks between the default wrap path and transparent +// redirect, so run.go has one call site and no build tags. + +// setgidHelperPath is the installed helper that adopts the sandbox group. +// Fixed, never resolved through PATH: a shadowed helper must not become the +// mechanism that decides whether traffic is governed. +const setgidHelperPath = "/usr/local/bin/onecli-sandbox-gid" + +func resolveEnforceWrapMode(env map[string]string) ( + profilePath string, port uint16, sess *transparentSession, err error, +) { + if transparentRequested() { + return resolveEnforceWrapTransparent(env) + } + profilePath, port, err = resolveEnforceWrap(env) + return profilePath, port, nil, err +} diff --git a/cmd/onecli/run_enforce_wrap_mode_other.go b/cmd/onecli/run_enforce_wrap_mode_other.go new file mode 100644 index 0000000..8c8d0fb --- /dev/null +++ b/cmd/onecli/run_enforce_wrap_mode_other.go @@ -0,0 +1,30 @@ +//go:build !darwin + +package main + +// Non-darwin builds have no transparent redirect: it depends on pf, which is +// macOS-specific. The type exists so run.go compiles unchanged; the Linux +// equivalent (nftables REDIRECT in a netns) is a separate piece of work. + +type transparentSession struct{} + +func (s *transparentSession) Close() error { return nil } + +// setgidHelperPath has no non-darwin equivalent yet; transparentSess is +// always nil here so it is never used. +const setgidHelperPath = "" + +func transparentWrapArgv(argv []string) []string { return argv } + +// The transparent sidecar has no non-darwin implementation; these keep +// main.go free of build tags. +func parseTransparentSidecarArgs([]string) (int, bool) { return 0, false } + +func runTransparentSidecar(int) {} + +func resolveEnforceWrapMode(env map[string]string) ( + profilePath string, port uint16, sess *transparentSession, err error, +) { + profilePath, port, err = resolveEnforceWrap(env) + return profilePath, port, nil, err +} diff --git a/cmd/onecli/run_enforce_wrap_test.go b/cmd/onecli/run_enforce_wrap_test.go index 62e9fbd..f63464f 100644 --- a/cmd/onecli/run_enforce_wrap_test.go +++ b/cmd/onecli/run_enforce_wrap_test.go @@ -24,6 +24,27 @@ func TestEnforceWrapLauncherGatesOnAvailability(t *testing.T) { } } +func TestGUIEditorsMarkedForEnforceRejection(t *testing.T) { + // `--enforce` fails closed for GUI editors: `onecli run -- cursor` only + // opens an Electron app, there's no launched process tree to sandbox, + // so wrap mode would sandbox-exec a launcher that governs nothing (and + // would inject the ephemeral forwarder port into the app's PERSISTENT + // settings.json). The routing keys on configDir != "", so pin that + // Cursor carries it — a regression here silently re-enables the broken + // path. + spec, ok := agentSkillDir("cursor") + if !ok { + t.Fatal("cursor spec not found") + } + if spec.configDir == "" { + t.Error("cursor must set configDir so --enforce fails closed for the GUI editor") + } + // A launched CLI agent (Codex) must NOT be treated as a GUI editor. + if codex, _ := agentSkillDir("codex"); codex.configDir != "" { + t.Error("codex is a CLI agent and must not carry configDir") + } +} + func TestRewriteProxyEnvToLoopback(t *testing.T) { env := map[string]string{ "HTTPS_PROXY": "http://x:aoc_tok@gateway.example.com:8443", @@ -61,7 +82,7 @@ func TestEnforceWrapArgvAppendsQuirks(t *testing.T) { // (e.g. sandbox-exec on macOS) is the sandbox package's concern and is // asserted there; here we assert only the platform-neutral contract: // the agent's own args are present and quirk flags are appended last. - got := strings.Join(enforceWrapArgv("/p/wrap.sb", "/bin/codex", []string{"exec", "task"}, "codex"), " ") + got := strings.Join(enforceWrapArgv("/p/wrap.sb", "/bin/codex", []string{"exec", "task"}, "codex", 0), " ") if !strings.Contains(got, "exec task") { t.Errorf("agent args missing from argv: %q", got) } @@ -72,7 +93,7 @@ func TestEnforceWrapArgvAppendsQuirks(t *testing.T) { func TestEnforceWrapNoticeAndQuirks(t *testing.T) { // An agent without quirks gets neither extra args nor a notice. - if len(enforceWrapQuirkArgs("somecli")) != 0 { + if len(enforceWrapQuirkArgs("somecli", 0)) != 0 { t.Error("no quirk args expected for an agent without quirks") } if enforceWrapNotice("codex") == "" { diff --git a/cmd/onecli/run_enforce_wrap_transparent_darwin.go b/cmd/onecli/run_enforce_wrap_transparent_darwin.go new file mode 100644 index 0000000..e21891f --- /dev/null +++ b/cmd/onecli/run_enforce_wrap_transparent_darwin.go @@ -0,0 +1,96 @@ +//go:build darwin + +package main + +// Wiring transparent redirect into `onecli run --enforce`. +// +// Opt-in via ONECLI_ENFORCE_TRANSPARENT=1 rather than on by default. The +// default wrap path denies direct egress at the OS layer and needs no +// privileged setup; transparent mode trades that for pf-layer governance so +// apps that ignore proxy configuration can still be governed. That is the +// right trade for a GUI editor and the wrong one for everything else, so the +// caller states it explicitly. +// +// The two artifacts must be installed together or not at all: the widened +// Seatbelt profile is only safe while the anchor is loaded. resolveEnforce- +// WrapTransparent either returns both or returns an error. + +import ( + "fmt" + "os" + + "github.com/onecli/onecli-cli/internal/sandbox" +) + +// enforceTransparentEnv opts a run into transparent redirect. +const enforceTransparentEnv = "ONECLI_ENFORCE_TRANSPARENT" + +// transparentRequested reports whether the caller asked for transparent mode. +func transparentRequested() bool { + return os.Getenv(enforceTransparentEnv) == "1" +} + +// resolveEnforceWrapTransparent prepares wrap mode WITH transparent redirect. +// +// Returns the sandbox profile, the CONNECT forwarder port (still used by +// apps that DO honor proxy env), and the live session owning the pf anchor. +// The caller must Close the session when the run ends. +// +// Fails closed: any error leaves nothing installed. In particular the +// widened profile is only materialized AFTER the anchor is verified live, +// so a failure can never leave a profile permitting direct 443 with no pf +// rules behind it. +func resolveEnforceWrapTransparent(env map[string]string) ( + profilePath string, port uint16, sess *transparentSession, err error, +) { + if err := sandbox.Available(); err != nil { + return "", 0, nil, err + } + gatewayURL := firstProxyURL(env) + if gatewayURL == "" { + return "", 0, nil, fmt.Errorf("no gateway proxy URL in the resolved environment") + } + + // The transparent session verifies setup, binds its listener, and loads + // the anchor. It refuses if any precondition is missing. + sess, err = startTransparentSession(gatewayURL) + if err != nil { + return "", 0, nil, fmt.Errorf("transparent redirect unavailable: %w", err) + } + + // The CONNECT forwarder still runs: apps that honor proxy env should + // use it directly rather than taking the redirect path, and Chromium's + // main process already does. + port, err = spawnEnforceForwarder(gatewayURL) + if err != nil { + _ = sess.Close() + return "", 0, nil, err + } + rewriteProxyEnvToLoopback(env, port) + + // Only now, with the anchor verified live, is the widened profile safe. + profilePath, err = sandbox.MaterializeOpts(sandbox.Options{ + ForwarderPort: port, + Transparent: true, + }) + if err != nil { + _ = sess.Close() + return "", 0, nil, err + } + return profilePath, port, sess, nil +} + +// transparentWrapArgv prefixes the sandbox launcher with the setgid helper +// so the confined tree runs under the group pf redirects. +// +// Order is load-bearing and was verified both ways: +// +// helper -> sandbox-exec -> cmd gid 700, works +// sandbox-exec -> helper -> cmd execvp: Operation not permitted +// +// Seatbelt refuses to exec a setgid binary from inside the sandbox, so the +// helper must run OUTSIDE it. The resulting group is inherited by the whole +// confined tree, which is what pf matches. +func transparentWrapArgv(argv []string) []string { + return append([]string{setgidHelperPath}, argv...) +} diff --git a/cmd/onecli/run_test.go b/cmd/onecli/run_test.go index 99280a6..5c5eb73 100644 --- a/cmd/onecli/run_test.go +++ b/cmd/onecli/run_test.go @@ -2,8 +2,14 @@ package main import ( "bytes" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" "encoding/json" + "encoding/pem" "io" + "math/big" "os" "os/exec" "path/filepath" @@ -11,6 +17,7 @@ import ( "slices" "strings" "testing" + "time" "github.com/onecli/onecli-cli/pkg/output" @@ -146,13 +153,17 @@ func TestAgentSkillDir(t *testing.T) { ok bool }{ {"claude", agentSpec{agentName: "Claude Code", baseDir: ".claude"}, true}, - {"cursor", agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor"}, true}, - {"agent", agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor"}, true}, + {"cursor", agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor", appBundle: "Cursor"}, true}, + {"agent", agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor", appBundle: "Cursor"}, true}, + // The headless Cursor agent is a launched CLI process (no configDir), + // so it's enforceable via the OS wrap, unlike the GUI launcher above. + {"cursor-agent", agentSpec{agentName: "Cursor Agent", baseDir: ".cursor"}, true}, + {"/usr/local/bin/cursor-agent", agentSpec{agentName: "Cursor Agent", baseDir: ".cursor"}, true}, {"codex", agentSpec{agentName: "Codex", baseDir: ".agents", skipHook: true, nativeProxyConfig: ".codex"}, true}, {"hermes", agentSpec{agentName: "Hermes", baseDir: ".hermes", skipHook: true, pluginGateway: true, dockerSandbox: true}, true}, {"opencode", agentSpec{agentName: "OpenCode", baseDir: ".opencode"}, true}, {"openclaw", agentSpec{agentName: "OpenClaw", baseDir: ".openclaw", skipHook: true, needsAnthropicKey: true}, true}, - {"/usr/local/bin/cursor", agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor"}, true}, + {"/usr/local/bin/cursor", agentSpec{agentName: "Cursor", baseDir: ".cursor", configDir: "Cursor", appBundle: "Cursor"}, true}, {"unknown", agentSpec{}, false}, } for _, tt := range tests { @@ -1058,3 +1069,149 @@ func writeJSON(t *testing.T, path, content string) { t.Fatal(err) } } + +// makeTestCA returns a self-signed CA PEM with the given common name. +// Each call generates a fresh key, so two calls with the SAME name model +// exactly the rotation that broke Cursor: identical subject, different key. +func makeTestCA(t *testing.T, commonName string) []byte { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: commonName}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("creating cert: %v", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +// The bug this guards: a rotated gateway CA keeps the SAME subject name, so +// every name-based check (security find-certificate, dump-trust-settings) +// reports the CA as present and trusted while Chromium rejects every request +// with ERR_CERT_AUTHORITY_INVALID. Only the key distinguishes them. +func TestPublicKeyOfPEM_DistinguishesRotatedCAWithIdenticalName(t *testing.T) { + const name = "OneCLI Local Gateway CA" + oldCA := makeTestCA(t, name) + newCA := makeTestCA(t, name) + + oldKey, err := publicKeyOfPEM(oldCA) + if err != nil { + t.Fatalf("parsing old CA: %v", err) + } + newKey, err := publicKeyOfPEM(newCA) + if err != nil { + t.Fatalf("parsing new CA: %v", err) + } + + if bytes.Equal(oldKey, newKey) { + t.Fatal("rotated CAs with the same subject name compared equal: the staleness check cannot work") + } + + // Same certificate must compare equal, or the check would warn on every + // launch and be trained away as noise. + sameKey, err := publicKeyOfPEM(oldCA) + if err != nil { + t.Fatalf("re-parsing old CA: %v", err) + } + if !bytes.Equal(oldKey, sameKey) { + t.Fatal("the same CA did not compare equal to itself") + } +} + +// The keychain returns every matching cert concatenated, and after a rotation +// that legitimately includes several. The live CA must be found among them. +func TestSplitPEMCerts_FindsLiveCAAmongStaleOnes(t *testing.T) { + const name = "OneCLI Local Gateway CA" + stale1 := makeTestCA(t, name) + stale2 := makeTestCA(t, name) + live := makeTestCA(t, name) + + var keychain bytes.Buffer + keychain.Write(stale1) + keychain.Write(stale2) + keychain.Write(live) + + blocks := splitPEMCerts(keychain.Bytes()) + if len(blocks) != 3 { + t.Fatalf("expected 3 certs, got %d", len(blocks)) + } + + liveKey, err := publicKeyOfPEM(live) + if err != nil { + t.Fatalf("parsing live CA: %v", err) + } + found := false + for _, b := range blocks { + if k, err := publicKeyOfPEM(b); err == nil && bytes.Equal(k, liveKey) { + found = true + } + } + if !found { + t.Fatal("live CA not found among the keychain certs: a valid setup would be reported as stale") + } + + // And a CA that is NOT installed must not be found, or the check would + // stay silent in exactly the broken case it exists to catch. + notInstalled := makeTestCA(t, name) + notInstalledKey, err := publicKeyOfPEM(notInstalled) + if err != nil { + t.Fatalf("parsing uninstalled CA: %v", err) + } + for _, b := range blocks { + if k, err := publicKeyOfPEM(b); err == nil && bytes.Equal(k, notInstalledKey) { + t.Fatal("an uninstalled CA was reported as present") + } + } +} + +// The bare CA file must track the bundle, since it is what the documented +// `security add-trusted-cert` command installs. A stale copy here is what +// caused the original failure. +func TestWriteBareGatewayCA_RefreshesOnRotation(t *testing.T) { + dir := t.TempDir() + oldCA := makeTestCA(t, "OneCLI Local Gateway CA") + newCA := makeTestCA(t, "OneCLI Local Gateway CA") + + if err := writeBareGatewayCA(dir, string(oldCA)); err != nil { + t.Fatalf("writing old CA: %v", err) + } + if err := writeBareGatewayCA(dir, string(newCA)); err != nil { + t.Fatalf("writing new CA: %v", err) + } + + got, err := os.ReadFile(filepath.Join(dir, "gateway-ca.pem")) + if err != nil { + t.Fatalf("reading CA file: %v", err) + } + if !bytes.Equal(got, newCA) { + t.Fatal("gateway-ca.pem still holds the pre-rotation CA: trusting it would fail as ERR_CERT_AUTHORITY_INVALID") + } +} + +// An empty CA (unauthenticated/offline session) must not truncate a good file. +func TestWriteBareGatewayCA_EmptyPEMLeavesFileIntact(t *testing.T) { + dir := t.TempDir() + good := makeTestCA(t, "OneCLI Local Gateway CA") + if err := writeBareGatewayCA(dir, string(good)); err != nil { + t.Fatalf("writing CA: %v", err) + } + if err := writeBareGatewayCA(dir, " "); err != nil { + t.Fatalf("empty write returned error: %v", err) + } + got, err := os.ReadFile(filepath.Join(dir, "gateway-ca.pem")) + if err != nil { + t.Fatalf("reading CA file: %v", err) + } + if !bytes.Equal(got, good) { + t.Fatal("an empty CA overwrote a valid one") + } +} diff --git a/cmd/onecli/sandbox_audit.go b/cmd/onecli/sandbox_audit.go index 8c55e44..d019198 100644 --- a/cmd/onecli/sandbox_audit.go +++ b/cmd/onecli/sandbox_audit.go @@ -512,7 +512,8 @@ func firstLine(b []byte) string { // SandboxCmd groups sandbox inspection commands. type SandboxCmd struct { - Audit SandboxAuditCmd `cmd:"" help:"Red-team the enforce-mode sandbox: try every known bypass and report which the OS actually stops."` + Audit SandboxAuditCmd `cmd:"" help:"Red-team the enforce-mode sandbox: try every known bypass and report which the OS actually stops."` + Transparent SandboxTransparentCmd `cmd:"" help:"Transparent redirect: govern apps that ignore proxy configuration (macOS)."` } // SandboxAuditCmd runs the escape matrix against a real profile. diff --git a/cmd/onecli/sandbox_transparent_cmd_darwin.go b/cmd/onecli/sandbox_transparent_cmd_darwin.go new file mode 100644 index 0000000..de6131e --- /dev/null +++ b/cmd/onecli/sandbox_transparent_cmd_darwin.go @@ -0,0 +1,96 @@ +//go:build darwin + +package main + +// `onecli sandbox transparent` — status and setup for transparent redirect. +// +// This is the user-facing entry point for the one privileged step. Before it +// existed, the readiness check told users to run a command that did not +// exist and setup only worked by hand-generating a script — a gap found by +// auditing which functions had non-test callers. +// +// Deliberately split into two subcommands: +// status reports readiness and needs no privilege +// setup PRINTS the script rather than executing it +// +// setup does not run itself under sudo on the user's behalf. It creates a +// group and edits sudoers, and a security tool that does that invisibly has +// not earned the trust it is asking for. Printing the exact commands lets +// the operator read them first, and makes the change auditable and +// reversible. + +import ( + "fmt" + "os" + "os/user" + + "github.com/onecli/onecli-cli/pkg/output" +) + +// SandboxTransparentCmd groups the transparent-redirect subcommands. +type SandboxTransparentCmd struct { + Status SandboxTransparentStatusCmd `cmd:"" help:"Report whether transparent redirect is ready to use."` + Setup SandboxTransparentSetupCmd `cmd:"" help:"Print the one-time privileged setup script (does not run it)."` +} + +// SandboxTransparentStatusCmd reports readiness. +type SandboxTransparentStatusCmd struct{} + +// Run prints the readiness report. +func (c *SandboxTransparentStatusCmd) Run() error { + printTransparentStatus(os.Stdout) + return nil +} + +// SandboxTransparentSetupCmd prints the privileged setup script. +type SandboxTransparentSetupCmd struct { + Helper string `help:"Path to the setgid helper source (defaults to the copy shipped with this checkout)." default:""` +} + +// Run renders the setup script for the operator to review and execute. +func (c *SandboxTransparentSetupCmd) Run() error { + out := output.New() + + if err := verifyTransparentSetup(); err == nil { + out.Stderr("onecli: transparent redirect is already set up. " + + "Run `onecli sandbox transparent status` to confirm.") + return nil + } + + gid, err := sandboxGID(transparentGroupName) + if err != nil { + gid, err = nextFreeGID() + if err != nil { + return fmt.Errorf("choosing a group ID: %w", err) + } + } + u, err := user.Current() + if err != nil { + return fmt.Errorf("resolving the current user: %w", err) + } + + fmt.Print(transparentSetupScript(gid, u.Username)) + fmt.Println() + fmt.Println("# Review the above, then run it:") + fmt.Printf("# onecli sandbox transparent setup | sudo bash\n") + fmt.Println("#") + fmt.Println("# Then install the setgid helper (needed because pf matches the") + fmt.Println("# EFFECTIVE gid, which an unprivileged process cannot adopt):") + fmt.Printf("# %s\n", helperInstallHint(c.Helper)) + return nil +} + +// helperInstallHint renders the compile-and-install line for the setgid +// helper, which cannot live in the shell script above because it needs the +// C source path from this checkout. +func helperInstallHint(srcOverride string) string { + src := srcOverride + if src == "" { + src = "internal/sandbox/helper/onecli-sandbox-gid.c" + } + return fmt.Sprintf( + "sudo sh -c 'cc -O2 -Wall -Wextra -Werror -DONECLI_SANDBOX_GID=$(dscl . -read /Groups/%s PrimaryGroupID | awk \"{print \\$2}\") "+ + "-o %s %s && chown root:%s %s && chmod 2755 %s'", + transparentGroupName, setgidHelperPath, src, + transparentGroupName, setgidHelperPath, setgidHelperPath) +} diff --git a/cmd/onecli/sandbox_transparent_cmd_darwin_test.go b/cmd/onecli/sandbox_transparent_cmd_darwin_test.go new file mode 100644 index 0000000..f9a45ae --- /dev/null +++ b/cmd/onecli/sandbox_transparent_cmd_darwin_test.go @@ -0,0 +1,88 @@ +//go:build darwin + +package main + +// Tests for the `onecli sandbox transparent` commands and the anchor +// reachability logic behind them. +// +// This file replaces an earlier scaffolding test whose only job was to +// PRINT the setup script, because no CLI command existed to do it. That +// gap (setup reachable only from tests, and an error message naming a +// command that did not exist) was found by auditing which functions had +// non-test callers, and is what these commands fix. + +import ( + "strings" + "testing" +) + +// TestAnchorIsReachableUnderstandsNesting is the regression guard for a bug +// that cost real debugging time: our anchor is "com.apple/onecli" and the +// stock macOS ruleset only contains `anchor "com.apple/*"`. An exact-name +// search reported the anchor as unreachable and refused to run, even though +// the wildcard covers it. +func TestAnchorIsReachableUnderstandsNesting(t *testing.T) { + stockRuleset := `scrub-anchor "com.apple/*" all fragment reassemble +anchor "com.apple/*" all +nat-anchor "com.apple/*" all +rdr-anchor "com.apple/*" all` + + for name, tc := range map[string]struct { + ruleset string + anchor string + want bool + }{ + "parent wildcard covers a nested anchor": {stockRuleset, "com.apple/onecli", true}, + "exact name": {`anchor "onecli" all`, "onecli", true}, + "global wildcard": {`anchor "*" all`, "onecli", true}, + "top-level anchor NOT covered": {stockRuleset, "onecli", false}, + "unrelated wildcard": {`anchor "com.other/*" all`, "com.apple/onecli", false}, + "empty ruleset": {"", "com.apple/onecli", false}, + } { + t.Run(name, func(t *testing.T) { + if got := anchorIsReachable(tc.ruleset, tc.anchor); got != tc.want { + t.Fatalf("anchorIsReachable(%q) = %v, want %v", tc.anchor, got, tc.want) + } + }) + } +} + +// TestTransparentSetupCommandNamesRealCommands: the setup output tells the +// operator what to run next, and every command it names must exist. The +// previous error message pointed at `onecli enforce setup-transparent`, +// which was never implemented. +func TestTransparentSetupCommandNamesRealCommands(t *testing.T) { + err := verifyTransparentSetup() + if err == nil { + t.Skip("machine is set up; the guidance path is not exercised") + } + if strings.Contains(err.Error(), "onecli enforce") { + t.Fatalf("error names a command that does not exist: %v", err) + } +} + +// TestHelperInstallHintIsComplete: the hint is the only place the setgid +// helper's install is documented in the product, so it must carry every +// step. A missing chmod would leave a binary without the setgid bit, which +// fails closed but leaves the user stuck. +func TestHelperInstallHintIsComplete(t *testing.T) { + hint := helperInstallHint("") + for _, required := range []string{ + "cc ", "-DONECLI_SANDBOX_GID=", "chown root:", "chmod 2755", + setgidHelperPath, transparentGroupName, + } { + if !strings.Contains(hint, required) { + t.Fatalf("install hint is missing %q:\n%s", required, hint) + } + } +} + +// TestTransparentStatusCommandRuns exercises the status path end to end. +// It must never error, whatever the machine's state: a status command that +// fails when things are unconfigured is useless precisely when it is needed. +func TestTransparentStatusCommandRuns(t *testing.T) { + cmd := &SandboxTransparentStatusCmd{} + if err := cmd.Run(); err != nil { + t.Fatalf("status command failed: %v", err) + } +} diff --git a/cmd/onecli/sandbox_transparent_cmd_other.go b/cmd/onecli/sandbox_transparent_cmd_other.go new file mode 100644 index 0000000..9741e27 --- /dev/null +++ b/cmd/onecli/sandbox_transparent_cmd_other.go @@ -0,0 +1,37 @@ +//go:build !darwin + +package main + +// Transparent redirect is macOS-only (it depends on pf). These stubs keep +// the command tree identical across platforms so help output and tests do +// not diverge; both subcommands explain why they are unavailable rather +// than silently missing. + +import ( + "fmt" + "runtime" +) + +// SandboxTransparentCmd groups the transparent-redirect subcommands. +type SandboxTransparentCmd struct { + Status SandboxTransparentStatusCmd `cmd:"" help:"Report whether transparent redirect is ready to use."` + Setup SandboxTransparentSetupCmd `cmd:"" help:"Print the one-time privileged setup script (does not run it)."` +} + +// SandboxTransparentStatusCmd reports readiness. +type SandboxTransparentStatusCmd struct{} + +// Run explains that the platform is unsupported. +func (c *SandboxTransparentStatusCmd) Run() error { + return fmt.Errorf("transparent redirect requires macOS pf; this is %s", runtime.GOOS) +} + +// SandboxTransparentSetupCmd prints the privileged setup script. +type SandboxTransparentSetupCmd struct { + Helper string `help:"Path to the setgid helper source." default:""` +} + +// Run explains that the platform is unsupported. +func (c *SandboxTransparentSetupCmd) Run() error { + return fmt.Errorf("transparent redirect requires macOS pf; this is %s", runtime.GOOS) +} diff --git a/docs/cursor-demo-runbook.md b/docs/cursor-demo-runbook.md new file mode 100644 index 0000000..c011da9 --- /dev/null +++ b/docs/cursor-demo-runbook.md @@ -0,0 +1,158 @@ +# OneCLI — Cursor coverage demo runbook + +Last verified: 2026-07-31 (all checks below were run and observed, not assumed). + +--- + +## The claim you can make + +> "OneCLI governs Cursor coding agents at the OS level. Not settings the app can +> ignore — kernel rules it cannot override. Both surfaces: the terminal agent and +> the IDE itself." + +--- + +## Before the demo + +1. **Merge + deploy** `onecli-cloud` PR #754, and set on the gateway: + ``` + GATEWAY_TUNNEL_HOSTS=*.cursor.sh + ``` + Without this, Cursor's in-IDE AI chat errors (its gRPC/HTTP-2 traffic cannot + traverse the HTTP/1.1-only MITM). Everything else works without it. + + Use the wildcard, not an enumerated host list. Tried and failed: + `api2,api3,api4,repo42.cursor.sh` looks complete from a packet capture but + misses `agentn.global.api5.cursor.sh`, which the agent loop uses and which is + even stricter than the rest — over HTTP/1.1 it does not answer at all + (`status=000`, connection refused) rather than returning an error status. + Cursor routes across regional and per-service subdomains that change without + notice, so any hand-written list is a latent outage. +2. **Merge** `onecli-cli` PR #112 (GUI sandbox launch, cursor-agent support). +3. **Trust the gateway CA** on the demo machine, Chromium ignores the env-var CA: + ``` + # Delete first: a CA trusted for an EARLIER gateway has the same name but a + # different key, and adding a second one does not fix it. + security delete-certificate -c "OneCLI Local Gateway CA" 2>/dev/null + + # NOTE: no -d. That selects the ADMIN trust domain, which needs root; run as + # a normal user it adds the certificate and applies NO trust settings, so the + # cert looks installed and every TLS handshake still fails. + security add-trusted-cert -r trustRoot \ + -k ~/Library/Keychains/login.keychain-db ~/.onecli/gateway-ca.pem + ``` + `~/.onecli/gateway-ca.pem` is refreshed on every authenticated run, so it is + always the CA the current gateway uses. + + Confirm trust was actually applied (presence is not trust): + ``` + security dump-trust-settings | grep "OneCLI Local Gateway CA" + ``` + No output means it is installed but untrusted, and GUI editors will still fail. + + **Verify it took** — this is the single highest-value pre-demo check, because + a stale CA fails in a way that looks like every name-based check passing: + ``` + onecli run --enforce -- cursor + ``` + If the trusted CA is not the live one, OneCLI now prints a warning naming the + exact commands to fix it. If you see `net::ERR_CERT_AUTHORITY_INVALID` in + `~/Library/Application Support/Cursor/logs//main.log`, that is this + problem: the CA rotated after you trusted it. `security find-certificate` and + `dump-trust-settings` will both still look correct, since only the key differs. +4. **Refresh vault credentials** for any agent you'll demo (Codex/Cursor tokens + expire; a dead token looks like a product failure on stage). +5. **Quit Cursor** — macOS focuses an existing window instead of launching a + sandboxed one, and OneCLI will (correctly) refuse. + +--- + +## Demo 1 — the terminal agent (strongest, fully unbypassable) + +```bash +onecli run --enforce -- cursor-agent +``` + +Then inside it, ask the agent to run: +```bash +curl -sS https://api.github.com/zen # works — via the gateway +curl --noproxy '*' --max-time 5 http://1.1.1.1/ # REFUSED by the OS in ms +``` + +**The point:** the second command fails in *milliseconds* (connect refused), not +by timeout. The agent cannot opt out — it has no route to the internet except the +gateway. + +--- + +## Demo 2 — the IDE itself + +```bash +onecli run --enforce -- cursor +``` + +Cursor opens **inside the sandbox**. Show, in this order: + +1. **Its own traffic is governed.** In a second terminal: + ```bash + lsof -nP -iTCP: # port is printed by onecli on launch + ``` + You'll see the editor's live connections bridged out to the gateway. +2. **Use the AI chat.** (Requires step 1 of "Before the demo".) +3. **Try to escape** from Cursor's integrated terminal: + ```bash + curl --noproxy '*' --max-time 5 http://1.1.1.1/ # refused by the OS + ``` +4. **Show the dashboard** — app.onecli.sh → Activity — with the requests listed. + +--- + +## Demo 3 — the red-team proof (do this one, it lands) + +```bash +onecli sandbox audit +``` + +Runs every known escape technique against the real profile and reports whether +the OS actually stopped it: direct dial, proxy-env stripped, raw sockets, child +processes, IPv6, UDP/DNS exfil, LaunchServices `open`, AppleEvents, the Docker +socket, and the deferred-egress vectors (planting a LaunchAgent, writing +`.zshrc`, dropping a binary on `$PATH`, rewriting `~/.onecli`, reading SSH keys). + +Expected: **PASS — no bypasses found**, plus the legitimate-capability checks +still working (DNS, git, home-cache writes), because a sandbox that breaks real +work gets switched off. + +--- + +## What to say honestly if asked + +- **Tunnelled hosts** (`GATEWAY_TUNNEL_HOSTS`) are the one real caveat, so state + it plainly. They keep the two things that matter most for the demo's claim: + egress is still confined to the gateway (the sandbox gives the agent no other + route out), and the connection still requires a valid agent token, so it is + attributed to a named agent/project/org. What they lose is everything that + depends on reading the stream: credential injection, content inspection, + per-request policy rules and rate limits, and per-request `request_logs` rows. + Verified directly: an invalid token on a tunnelled host is still rejected 407. + Scope the list to one vendor's domain (`*.cursor.sh` matches only subdomains of + `cursor.sh`, never `evilcursor.sh`); never tunnel a domain you don't control the + reason for. Full HTTP/2 MITM support is the planned fix that removes the caveat. +- **Enforcement covers agents OneCLI launches.** A developer who runs an agent + outside OneCLI isn't sandboxed — that's what the enrollment/attestation layer + on the roadmap addresses (PATH shims, coverage reporting). +- **Interactive OAuth logins** (`cursor-agent login`) need a browser, which + enforce blocks by design. Authenticate once outside enforce, then enforce. + +--- + +## Known-good vs blocked (as of last verification) + +| Check | Status | +|---|---| +| `onecli sandbox audit` | ✅ PASS, all vectors refused | +| enforce-wrapped bash → gateway | ✅ 200 | +| bypass attempt from inside the wrap | ✅ refused by OS in 17ms | +| `cursor-agent` recognized + wrapped | ✅ | +| Cursor GUI launches sandboxed | ✅ 0 errors, 10 live gateway connections | +| Cursor in-IDE AI chat | ⏳ needs PR #754 deployed + `GATEWAY_TUNNEL_HOSTS` | diff --git a/internal/sandbox/helper/onecli-sandbox-gid.c b/internal/sandbox/helper/onecli-sandbox-gid.c new file mode 100644 index 0000000..46d0e90 --- /dev/null +++ b/internal/sandbox/helper/onecli-sandbox-gid.c @@ -0,0 +1,102 @@ +/* + * onecli-sandbox-gid: adopt the OneCLI sandbox group, then exec. + * + * Why this exists: pf scopes the transparent redirect by GID, and it matches + * the socket's EFFECTIVE group. Measured on this machine: a shell that has + * onecli-sandbox as a SUPPLEMENTARY group was not matched by the anchor's + * rules, so supplementary membership is not enough. POSIX only lets an + * unprivileged process adopt its real or saved GID, so something must set + * the effective GID at exec: that is what the setgid bit does. + * + * Written in C rather than Go deliberately. This is a setgid binary, so its + * attack surface should be as close to nothing as possible: no runtime, no + * threads spawned before main, no environment-driven behavior. + * + * SECURITY REVIEW, stated plainly: + * - It grants GID onecli-sandbox, which owns no files and confers no + * privileges. Under the loaded pf anchor that group is MORE restricted + * than a normal one (default-deny egress). Running this is strictly a + * downgrade, which is why it is safe for any user to execute. + * - It does NOT setuid. The caller's user identity is unchanged. + * - It sets the REAL gid as well as the effective one, so no code can + * later restore the original group via setgid(getgid()). + * - It drops all supplementary groups, otherwise the original groups + * would remain in the credential set. + * - It refuses to run if the setgid bit did not take effect, rather than + * silently exec'ing ungoverned. That failure is the whole ballgame: a + * process that runs with the wrong GID is not redirected by pf, and + * with the transparent Seatbelt profile it would have direct egress. + */ + +#include +#include /* setgroups() decl; see note in main() about why it is unused */ +#include +#include +#include +#include + +#ifndef ONECLI_SANDBOX_GID +#error "ONECLI_SANDBOX_GID must be defined at compile time" +#endif + +int main(int argc, char *argv[]) { + if (argc < 2) { + fprintf(stderr, "usage: onecli-sandbox-gid [args...]\n"); + return 2; + } + + gid_t target = (gid_t)ONECLI_SANDBOX_GID; + + /* The setgid bit should have made this our effective GID. If it did + * not, the binary is not installed correctly and we must not continue: + * the caller is about to run under a Seatbelt profile that permits + * direct 443, governed only by a pf rule keyed to this GID. */ + if (getegid() != target) { + fprintf(stderr, + "onecli: setgid bit not in effect (egid=%d, want %d); " + "refusing to run ungoverned\n", + (int)getegid(), (int)target); + return 1; + } + + /* Set the REAL gid as well, so nothing can later switch back via + * setgid(getgid()). + * + * setregid(), not setgid(): measured on macOS, an unprivileged + * setgid(target) whose egid is already target sets only the effective + * gid and leaves the real gid alone, producing gid=20 egid=700. The + * strict check below caught that rather than letting it through. + * setregid(target, target) sets both. */ + if (setregid(target, target) != 0) { + fprintf(stderr, "onecli: setregid failed: %s\n", strerror(errno)); + return 1; + } + + /* Deliberately NOT calling setgroups(): it requires root, and this + * binary is setgid only. Measured, which is why this comment exists + * rather than the call: setgroups(1, &target) fails with EPERM here. + * + * That leaves the caller's original supplementary groups in the + * credential set, and it does NOT weaken the guarantee. pf matches the + * EFFECTIVE gid, which is the property this binary exists to set, and + * that was established by direct measurement: with the anchor blocking + * gid 700, a shell holding onecli-sandbox as a SUPPLEMENTARY group + * still reached the internet. Supplementary groups are invisible to + * the rules that govern egress. + * + * Supplementary groups do affect FILE access, but file confinement is + * the Seatbelt profile's job, not this binary's, and the profile + * applies to the whole process tree regardless of group. */ + + /* Verify both ids before exec: a wrong GID here means an unredirected + * process running under a profile that permits direct 443. */ + if (getgid() != target || getegid() != target) { + fprintf(stderr, "onecli: gid did not stick (gid=%d egid=%d)\n", + (int)getgid(), (int)getegid()); + return 1; + } + + execvp(argv[1], &argv[1]); + fprintf(stderr, "onecli: exec %s failed: %s\n", argv[1], strerror(errno)); + return 127; +} diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index b028f76..bc5c149 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -38,3 +38,11 @@ func WrapArgv(profile, binary string, args []string) []string { // derive a deliberately-holed variant to prove the audit itself detects // holes; normal runs use Materialize. func Profile(forwarderPort uint16) string { return profile(forwarderPort) } + +// MaterializeOpts writes the profile for an explicit network posture. +// Prefer Materialize unless you are enabling transparent redirect, which +// requires a verified pf anchor to stay fail-closed. +func MaterializeOpts(opts Options) (string, error) { return materializeOpts(opts) } + +// ProfileOpts returns the profile text for an explicit posture. +func ProfileOpts(opts Options) string { return profileOpts(opts) } diff --git a/internal/sandbox/sandbox_darwin.go b/internal/sandbox/sandbox_darwin.go index 14d8782..a806655 100644 --- a/internal/sandbox/sandbox_darwin.go +++ b/internal/sandbox/sandbox_darwin.go @@ -82,10 +82,7 @@ const seatbeltProfileTemplate = `(version 1) ; the OneCLI gateway. Filesystem rules are limited to the surfaces that ; would hand an unsandboxed process the network on the agent's behalf. (allow default) -(deny network-outbound) -(allow network-outbound (remote unix-socket)) -(deny network-outbound (regex #"docker\.sock$")) -(allow network-outbound (remote tcp "localhost:{{PORT}}")) +{{NETWORK}} (deny lsopen) (deny appleevent-send) @@ -157,13 +154,67 @@ const seatbeltProfileTemplate = `(version 1) (deny file-read* (subpath "{{HOME}}/.aws")) ` +// transparentNetworkStanza replaces the loopback-only network rules when +// transparent redirect is active. +// +// Why this exists, and why it is NOT a weakening: Seatbelt adjudicates at +// connect(), in the socket layer, BEFORE any packet is emitted. Measured +// against unroutable addresses, a Seatbelt-denied connect returns EPERM in +// 16ms while an allowed one takes the full 6s network timeout. pf works on +// packets, so it can only redirect a connection Seatbelt permitted. To +// transparently proxy an app that dials directly — the Cursor extension +// host being the case that forced this — the profile must let the packet +// out so pf can divert it to the loopback listener. +// +// The governance that Seatbelt used to provide for port 443 moves to the pf +// anchor, which default-denies the sandbox group and re-permits only +// loopback, redirected 443, and DNS. Enforcement is therefore preserved, +// not traded away, but it now depends on the anchor being loaded — which is +// why enabling this mode REQUIRES a verified anchor (see +// requireTransparentAnchor in the caller) and refuses to run without one. +// +// Ports other than 443 stay denied at the Seatbelt layer, so this widens +// exactly the one port pf is configured to capture. +const transparentNetworkStanza = `(deny network-outbound) +(allow network-outbound (remote unix-socket)) +(deny network-outbound (regex #"docker\.sock$")) +(allow network-outbound (remote tcp "localhost:{{PORT}}")) +; Transparent redirect: pf diverts this to the loopback listener. Seatbelt +; must permit the connect() for a packet to exist for pf to act on. +(allow network-outbound (remote tcp "*:443"))` + +// loopbackNetworkStanza is the default: no direct egress at any port. +const loopbackNetworkStanza = `(deny network-outbound) +(allow network-outbound (remote unix-socket)) +(deny network-outbound (regex #"docker\.sock$")) +(allow network-outbound (remote tcp "localhost:{{PORT}}"))` + +// Options selects the network posture of the rendered profile. +type Options struct { + // ForwarderPort is the loopback listener the sandbox may reach. + ForwarderPort uint16 + // Transparent permits outbound 443 so a pf anchor can redirect it. + // Only set this when a verified anchor is loaded: without one, the + // permitted port becomes ungoverned direct egress. + Transparent bool +} + // profileFor renders the profile for a home directory. Exported through // Profile()/Materialize() so the audit and the enforced run share one // definition — two copies of a security policy is how the docker.sock // rule stayed broken while a text assertion passed. func profileFor(home string, forwarderPort uint16) string { - out := strings.ReplaceAll(seatbeltProfileTemplate, "{{HOME}}", home) - return strings.ReplaceAll(out, "{{PORT}}", strconv.Itoa(int(forwarderPort))) + return profileForOpts(home, Options{ForwarderPort: forwarderPort}) +} + +func profileForOpts(home string, opts Options) string { + network := loopbackNetworkStanza + if opts.Transparent { + network = transparentNetworkStanza + } + out := strings.Replace(seatbeltProfileTemplate, "{{NETWORK}}", network, 1) + out = strings.ReplaceAll(out, "{{HOME}}", home) + return strings.ReplaceAll(out, "{{PORT}}", strconv.Itoa(int(opts.ForwarderPort))) } // sandboxExecPath is where macOS ships sandbox-exec. A fixed path, not @@ -192,9 +243,21 @@ func profile(forwarderPort uint16) string { return profileFor(home, forwarderPort) } +func profileOpts(opts Options) string { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return profileForOpts(home, opts) +} + // materialize writes the Seatbelt profile under ~/.onecli and returns its // path, rewriting only when stale. func materialize(forwarderPort uint16) (string, error) { + return materializeOpts(Options{ForwarderPort: forwarderPort}) +} + +func materializeOpts(opts Options) (string, error) { home, err := os.UserHomeDir() if err != nil { return "", fmt.Errorf("resolving home dir: %w", err) @@ -203,8 +266,15 @@ func materialize(forwarderPort uint16) (string, error) { if err := os.MkdirAll(dir, 0o700); err != nil { return "", fmt.Errorf("creating profile dir: %w", err) } - rendered := profileFor(home, forwarderPort) - path := filepath.Join(dir, "enforce-wrap.sb") + rendered := profileForOpts(home, opts) + // Distinct filenames per mode: a transparent profile permits outbound + // 443 and is only safe with a loaded anchor, so it must never be + // picked up by a normal run through a stale shared path. + name := "enforce-wrap.sb" + if opts.Transparent { + name = "enforce-wrap-transparent.sb" + } + path := filepath.Join(dir, name) if existing, err := os.ReadFile(path); err == nil && string(existing) == rendered { return path, nil } diff --git a/internal/sandbox/sandbox_other.go b/internal/sandbox/sandbox_other.go index 7dceca6..a4a947a 100644 --- a/internal/sandbox/sandbox_other.go +++ b/internal/sandbox/sandbox_other.go @@ -18,6 +18,18 @@ func available() error { func materialize(uint16) (string, error) { return "", available() } +// Options mirrors the darwin type so callers compile everywhere. Transparent +// redirect is macOS-only today (it depends on pf); the Linux equivalent is +// an nftables REDIRECT inside the network namespace. +type Options struct { + ForwarderPort uint16 + Transparent bool +} + +func materializeOpts(Options) (string, error) { return "", available() } + +func profileOpts(Options) string { return "" } + func launcherPath() string { return "" } func profile(uint16) string { return "" } diff --git a/internal/sandbox/sandbox_transparent_darwin_test.go b/internal/sandbox/sandbox_transparent_darwin_test.go new file mode 100644 index 0000000..2cea7d8 --- /dev/null +++ b/internal/sandbox/sandbox_transparent_darwin_test.go @@ -0,0 +1,104 @@ +//go:build darwin + +package sandbox + +// Tests for the transparent-redirect profile variant. +// +// The security-critical property is that the DEFAULT posture is unchanged: +// transparent mode permits outbound 443 (so pf has a packet to redirect), +// and that permission must never leak into an ordinary enforced run, where +// no anchor exists to govern it. + +import ( + "strings" + "testing" +) + +// TestDefaultProfileDeniesDirect443 is the regression guard. If this ever +// fails, ordinary enforce runs have gained direct egress. +func TestDefaultProfileDeniesDirect443(t *testing.T) { + p := profileForOpts("/Users/probe", Options{ForwarderPort: 4242}) + if strings.Contains(p, `"*:443"`) { + t.Fatalf("the DEFAULT profile permits direct 443; enforce is no longer fail-closed:\n%s", p) + } + if !strings.Contains(p, "(deny network-outbound)") { + t.Fatal("the default profile lost its network deny") + } + if !strings.Contains(p, `(allow network-outbound (remote tcp "localhost:4242"))`) { + t.Fatal("the default profile lost its forwarder allow") + } +} + +// TestTransparentProfilePermits443 documents the deliberate widening. +func TestTransparentProfilePermits443(t *testing.T) { + p := profileForOpts("/Users/probe", Options{ForwarderPort: 4242, Transparent: true}) + if !strings.Contains(p, `(allow network-outbound (remote tcp "*:443"))`) { + t.Fatalf("transparent profile does not permit 443, so pf can never "+ + "see a packet to redirect:\n%s", p) + } + // The forwarder allow must survive: the redirect lands on loopback. + if !strings.Contains(p, `(allow network-outbound (remote tcp "localhost:4242"))`) { + t.Fatal("transparent profile lost its loopback allow") + } +} + +// TestTransparentProfileWidensOnly443 bounds the widening. Permitting any +// other port would create egress pf is not configured to capture, and the +// pf anchor's default-deny is the only thing that would catch it. +func TestTransparentProfileWidensOnly443(t *testing.T) { + base := profileForOpts("/Users/probe", Options{ForwarderPort: 4242}) + tp := profileForOpts("/Users/probe", Options{ForwarderPort: 4242, Transparent: true}) + + baseLines := map[string]bool{} + for _, l := range strings.Split(base, "\n") { + baseLines[strings.TrimSpace(l)] = true + } + var added []string + for _, l := range strings.Split(tp, "\n") { + t := strings.TrimSpace(l) + if t == "" || strings.HasPrefix(t, ";") || baseLines[t] { + continue + } + added = append(added, t) + } + if len(added) != 1 { + t.Fatalf("transparent mode changed %d rules, want exactly 1: %v", len(added), added) + } + if added[0] != `(allow network-outbound (remote tcp "*:443"))` { + t.Fatalf("unexpected rule added by transparent mode: %q", added[0]) + } +} + +// TestTransparentProfileKeepsEscapeDenials: the deferred-execution and +// credential-read denials are what stop a sandboxed process handing its work +// to an unsandboxed one. Transparent mode must not relax any of them. +func TestTransparentProfileKeepsEscapeDenials(t *testing.T) { + p := profileForOpts("/Users/probe", Options{ForwarderPort: 4242, Transparent: true}) + for _, required := range []string{ + `(deny lsopen)`, + `(deny appleevent-send)`, + `(deny network-outbound (regex #"docker\.sock$"))`, + `(deny file-write* (subpath "/Users/probe/Library/LaunchAgents"))`, + `(deny file-write* (regex #"/\.git/hooks/"))`, + `(deny file-read* (subpath "/Users/probe/.ssh"))`, + `(deny file-write* (literal "/Users/probe/.onecli/enforce-wrap.sb"))`, + } { + if !strings.Contains(p, required) { + t.Fatalf("transparent profile dropped a required denial: %s", required) + } + } +} + +// TestProfilesRenderNoPlaceholders: an unexpanded {{NETWORK}} or {{HOME}} +// would load cleanly and match nothing — the silent-no-op failure this +// package exists to prevent. +func TestProfilesRenderNoPlaceholders(t *testing.T) { + for name, p := range map[string]string{ + "default": profileForOpts("/Users/probe", Options{ForwarderPort: 4242}), + "transparent": profileForOpts("/Users/probe", Options{ForwarderPort: 4242, Transparent: true}), + } { + if strings.Contains(p, "{{") { + t.Fatalf("%s profile has an unexpanded placeholder:\n%s", name, p) + } + } +} diff --git a/internal/sandbox/sandbox_transparent_kernel_darwin_test.go b/internal/sandbox/sandbox_transparent_kernel_darwin_test.go new file mode 100644 index 0000000..1b086f7 --- /dev/null +++ b/internal/sandbox/sandbox_transparent_kernel_darwin_test.go @@ -0,0 +1,114 @@ +//go:build darwin + +package sandbox + +// A kernel-level check that both profile variants actually LOAD. +// +// Text assertions prove a profile says what we intended. They cannot prove +// the kernel accepts it: a profile with a syntax error, or a rule the +// sandbox rejects, fails only at launch. This package has been bitten by +// exactly that gap before (rules that loaded cleanly and matched nothing), +// so both variants are handed to the real sandbox-exec. + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + "time" +) + +func TestBothProfilesLoadInTheRealKernel(t *testing.T) { + if _, err := os.Stat(sandboxExecPath); err != nil { + t.Skipf("sandbox-exec unavailable: %v", err) + } + home, err := os.UserHomeDir() + if err != nil { + t.Fatalf("home: %v", err) + } + dir := t.TempDir() + + for name, opts := range map[string]Options{ + "default": {ForwarderPort: 4242}, + "transparent": {ForwarderPort: 4242, Transparent: true}, + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(dir, name+".sb") + if err := os.WriteFile(path, []byte(profileForOpts(home, opts)), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + // `true` does nothing; the point is whether the profile loads. + out, err := exec.Command(sandboxExecPath, "-f", path, "/usr/bin/true").CombinedOutput() + if err != nil { + t.Fatalf("the %s profile was REJECTED by the kernel: %v\n%s", name, err, out) + } + }) + } +} + +// TestTransparentProfileActuallyPermits443 proves the widening is real at +// the kernel level, not just present in the text. It uses a local listener +// on 443 rather than a remote host so the test needs no internet: what +// matters is whether Seatbelt permits the connect(), and a refused +// connection to a closed local port is still an ALLOWED syscall, whereas a +// Seatbelt denial surfaces differently. +// +// Distinguishing the two reliably is what makes this test meaningful, so it +// compares the two profiles against the SAME target: under the default +// profile the connect must fail, under transparent it must not fail in the +// same way. +func TestTransparentProfileActuallyPermits443(t *testing.T) { + if _, err := os.Stat(sandboxExecPath); err != nil { + t.Skipf("sandbox-exec unavailable: %v", err) + } + if os.Getenv("ONECLI_LIVE_TRANSPARENT") != "1" { + t.Skip("set ONECLI_LIVE_TRANSPARENT=1 (makes a real outbound connection)") + } + home, err := os.UserHomeDir() + if err != nil { + t.Fatalf("home: %v", err) + } + dir := t.TempDir() + + write := func(name string, opts Options) string { + p := filepath.Join(dir, name+".sb") + if err := os.WriteFile(p, []byte(profileForOpts(home, opts)), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + return p + } + defaultProfile := write("default", Options{ForwarderPort: 4242}) + transparentProfile := write("transparent", Options{ForwarderPort: 4242, Transparent: true}) + + // TEST-NET-1 never routes, so an ALLOWED connect hangs until timeout + // while a DENIED one returns immediately. That timing gap is the + // signal, and it is the same measurement that established Seatbelt + // adjudicates at connect(). + const target = "https://192.0.2.1/" + + run := func(profile string) (secs float64) { + start := nowSeconds() + cmd := exec.Command(sandboxExecPath, "-f", profile, + "curl", "-sS", "--max-time", "4", target) + _ = cmd.Run() + return nowSeconds() - start + } + + denied := run(defaultProfile) + allowed := run(transparentProfile) + t.Logf("connect to an unroutable 443: default=%.3fs transparent=%.3fs", denied, allowed) + + if denied > 1.0 { + t.Fatalf("the DEFAULT profile took %.3fs, so it did not deny the "+ + "connect; enforce may no longer be fail-closed", denied) + } + if allowed < 1.0 { + t.Fatalf("the TRANSPARENT profile returned in %.3fs, so the connect "+ + "was still denied and pf would never see a packet to redirect", allowed) + } +} + +// nowSeconds is a monotonic-ish clock for the timing comparison above. +func nowSeconds() float64 { + return float64(time.Now().UnixNano()) / 1e9 +}