diff --git a/cmd/flynn/mcp.go b/cmd/flynn/mcp.go index 38238eee..6e30c3a7 100644 --- a/cmd/flynn/mcp.go +++ b/cmd/flynn/mcp.go @@ -79,7 +79,10 @@ func serveMCP(ctx context.Context, dataDir, dir string, readOnly bool, in io.Rea // assembly is the point: a served session is governed by the same grant and records // onto the same kind of stream as the agent's own run, so authority cannot drift // between the two paths. - shell := governed.ShellPolicy{DataDir: dataDir, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true} + shell := governed.ShellPolicy{ + Limits: commandLimits(0, 0), + DataDir: dataDir, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true, + } if n := credentialNotice(shell); n != "" && !readOnly { _, _ = fmt.Fprintln(logw, n, "Serve with --read-only to expose no shell.") } diff --git a/cmd/flynn/shellpolicy.go b/cmd/flynn/shellpolicy.go index 305a99ca..6da3854a 100644 --- a/cmd/flynn/shellpolicy.go +++ b/cmd/flynn/shellpolicy.go @@ -1,10 +1,12 @@ package main import ( + "context" "flag" "fmt" "github.com/ionalpha/flynn/governed" + "github.com/ionalpha/flynn/internal/hardware" "github.com/ionalpha/flynn/sandbox" ) @@ -20,7 +22,7 @@ type shellFlags struct { // registerShellFlags declares the shell policy flags on fs. func registerShellFlags(fs *flag.FlagSet) shellFlags { return shellFlags{ - maxMemory: fs.Int("max-memory", 0, "cap the memory (MiB) a command the agent runs may commit; 0 (default) is unlimited. Bounds a memory bomb; enforced where the platform supports it (a Windows job object today)."), + maxMemory: fs.Int("max-memory", 0, "cap the memory (MiB) a command the agent runs may commit; 0 (default) caps it at half this machine's physical memory. Bounds a memory bomb or a command that keeps starting copies of itself; enforced where the platform supports it (a Windows job object today)."), maxProcs: fs.Int("max-processes", 0, "cap how many processes a command the agent runs may spawn; 0 (default) uses the platform's generous fork-bomb backstop."), allowNet: fs.Bool("allow-network", false, "give the commands the agent runs the host's network. By default they run with none, so a command cannot send what it reads anywhere; this is needed for git over the network, package installs and anything else a command fetches. Windows cannot deny the network, so there they have it unless --require-network-denial is passed. The choice is recorded on the run's stream."), allowCreds: fs.Bool("allow-credential-reads", false, "leave the Flynn data directory and the host's credential locations (~/.ssh, cloud and CLI login tokens, keychain sockets) readable to the commands the agent runs. By default they are hidden on Linux and macOS; Windows cannot hide a path from a command, so there they stay readable unless --require-credential-hiding is passed. The choice is recorded on the run's stream."), @@ -33,7 +35,7 @@ func registerShellFlags(fs *flag.FlagSet) shellFlags { // dataDir. func (f shellFlags) policy(dataDir string) governed.ShellPolicy { return governed.ShellPolicy{ - Limits: sandbox.ResourceLimits{MemoryMiB: *f.maxMemory, MaxProcesses: *f.maxProcs}, + Limits: commandLimits(*f.maxMemory, *f.maxProcs), DataDir: dataDir, AllowNetwork: *f.allowNet, AllowCredentialReads: *f.allowCreds, @@ -45,6 +47,29 @@ func (f shellFlags) policy(dataDir string) governed.ShellPolicy { } } +// hostRAMBytes reads the machine's physical memory. It is a variable so a test can give +// the default cap a known machine to be computed against. +var hostRAMBytes = hardware.RAMBytes + +// commandLimits is the resource cap on every command the agent runs, from the +// --max-memory and --max-processes values. A memory value of 0 does not mean unlimited: +// it means half the machine's physical memory. The sandbox's process cap alone does not +// protect the host. A command that keeps starting copies of itself (a PATH shim that +// resolves back to itself, for one) can commit all of RAM with a few hundred heavy +// processes, far below the process backstop, and take the operator's machine down with +// the run. Half leaves the host, and this process, room to stop the run and report it. +// +// The default is set here, on the agent's shell policy, and not inside [sandbox.Local], +// because the sandbox also runs the local model server, which can legitimately need more +// than half the machine's memory. Where the RAM cannot be read the command runs uncapped, +// as before. +func commandLimits(memoryMiB, maxProcesses int) sandbox.ResourceLimits { + if memoryMiB <= 0 { + memoryMiB = int(hostRAMBytes(context.Background()) / 2 >> 20) + } + return sandbox.ResourceLimits{MemoryMiB: memoryMiB, MaxProcesses: maxProcesses} +} + // networkNotice is what an operator is told before a run whose commands have the host's // network only because this platform cannot deny it, or "" for any other run. The posture // event records it with the reason. diff --git a/cmd/flynn/shellpolicy_limits_test.go b/cmd/flynn/shellpolicy_limits_test.go new file mode 100644 index 00000000..a448666a --- /dev/null +++ b/cmd/flynn/shellpolicy_limits_test.go @@ -0,0 +1,106 @@ +package main + +import ( + "context" + "go/ast" + "go/parser" + "go/token" + "os" + "strings" + "testing" + + "github.com/ionalpha/flynn/sandbox" +) + +// stubRAM makes [commandLimits] compute its default against a machine with ram bytes of +// physical memory, for the life of the test. +func stubRAM(t *testing.T, ram int64) { + t.Helper() + prev := hostRAMBytes + hostRAMBytes = func(context.Context) int64 { return ram } + t.Cleanup(func() { hostRAMBytes = prev }) +} + +// TestCommandLimitsDefaultsToHalfOfRAM pins the default memory cap: half the machine's +// physical memory when no cap was asked for, the asked-for cap when one was, and no cap +// only where the RAM cannot be read. +func TestCommandLimitsDefaultsToHalfOfRAM(t *testing.T) { + for _, tc := range []struct { + name string + ram int64 + memoryMiB, maxProc int + want sandbox.ResourceLimits + }{ + {"default is half of RAM", 64 << 30, 0, 0, sandbox.ResourceLimits{MemoryMiB: 32 << 10}}, + {"odd RAM rounds down", 3<<30 + 1, 0, 0, sandbox.ResourceLimits{MemoryMiB: 1536}}, + {"explicit cap wins", 64 << 30, 1024, 0, sandbox.ResourceLimits{MemoryMiB: 1024}}, + {"explicit cap above the default wins", 8 << 30, 6144, 0, sandbox.ResourceLimits{MemoryMiB: 6144}}, + {"negative is treated as unset", 8 << 30, -1, 0, sandbox.ResourceLimits{MemoryMiB: 4096}}, + {"process cap passes through", 8 << 30, 0, 64, sandbox.ResourceLimits{MemoryMiB: 4096, MaxProcesses: 64}}, + {"unknown RAM leaves memory uncapped", 0, 0, 0, sandbox.ResourceLimits{}}, + } { + t.Run(tc.name, func(t *testing.T) { + stubRAM(t, tc.ram) + if got := commandLimits(tc.memoryMiB, tc.maxProc); got != tc.want { + t.Fatalf("commandLimits(%d, %d) on %d bytes = %+v, want %+v", tc.memoryMiB, tc.maxProc, tc.ram, got, tc.want) + } + }) + } +} + +// TestCommandLimitsReadsThisMachine checks the real probe feeds the default, so a build +// where hardware.RAMBytes stopped working is caught rather than shipping uncapped +// commands. Every platform the binary ships for can read its RAM. +func TestCommandLimitsReadsThisMachine(t *testing.T) { + if got := commandLimits(0, 0); got.MemoryMiB <= 0 { + t.Fatalf("commandLimits(0, 0) = %+v on this machine, want a positive default memory cap", got) + } +} + +// TestEveryShellPolicySetsLimits holds the claim on [commandLimits] that every command the +// agent runs is capped: each governed.ShellPolicy this package builds outside its tests +// must set Limits, so a new entry point cannot start a shell with no memory cap. +func TestEveryShellPolicySetsLimits(t *testing.T) { + entries, err := os.ReadDir(".") + if err != nil { + t.Fatalf("read package directory: %v", err) + } + fset := token.NewFileSet() + found := 0 + for _, entry := range entries { + fname := entry.Name() + if entry.IsDir() || !strings.HasSuffix(fname, ".go") || strings.HasSuffix(fname, "_test.go") { + continue + } + file, err := parser.ParseFile(fset, fname, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", fname, err) + } + ast.Inspect(file, func(n ast.Node) bool { + lit, ok := n.(*ast.CompositeLit) + if !ok { + return true + } + sel, ok := lit.Type.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "ShellPolicy" { + return true + } + if x, ok := sel.X.(*ast.Ident); !ok || x.Name != "governed" { + return true + } + found++ + for _, elt := range lit.Elts { + if kv, ok := elt.(*ast.KeyValueExpr); ok { + if k, ok := kv.Key.(*ast.Ident); ok && k.Name == "Limits" { + return true + } + } + } + t.Errorf("%s: governed.ShellPolicy without Limits; set it with commandLimits so the commands it runs have a memory cap", fset.Position(lit.Pos())) + return true + }) + } + if found == 0 { + t.Fatal("found no governed.ShellPolicy literal; the check is no longer looking at the code that builds the policy") + } +} diff --git a/cmd/flynn/shellpolicy_test.go b/cmd/flynn/shellpolicy_test.go index ce409c3c..acf3c3b5 100644 --- a/cmd/flynn/shellpolicy_test.go +++ b/cmd/flynn/shellpolicy_test.go @@ -31,21 +31,28 @@ func useHome(t *testing.T) string { // TestShellFlagsBuildThePolicy shows the flags are the only way to relax the policy, and that // leaving them off leaves it closed wherever the platform can close it. func TestShellFlagsBuildThePolicy(t *testing.T) { + stubRAM(t, 8<<30) + capped := sandbox.ResourceLimits{MemoryMiB: 4096} for _, tc := range []struct { name string args []string want governed.ShellPolicy }{ - {"no flags", nil, governed.ShellPolicy{DataDir: "/d", AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}}, - {"network", []string{"-allow-network"}, governed.ShellPolicy{DataDir: "/d", AllowNetwork: true, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}}, - {"credentials", []string{"-allow-credential-reads"}, governed.ShellPolicy{DataDir: "/d", AllowCredentialReads: true, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}}, - {"strict credentials", []string{"-require-credential-hiding"}, governed.ShellPolicy{DataDir: "/d", AllowNetworkWhereUndeniable: true}}, - {"strict network", []string{"-require-network-denial"}, governed.ShellPolicy{DataDir: "/d", AllowCredentialReadsWhereUnhidable: true}}, + {"no flags", nil, governed.ShellPolicy{DataDir: "/d", Limits: capped, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}}, + {"network", []string{"-allow-network"}, governed.ShellPolicy{DataDir: "/d", Limits: capped, AllowNetwork: true, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}}, + {"credentials", []string{"-allow-credential-reads"}, governed.ShellPolicy{DataDir: "/d", Limits: capped, AllowCredentialReads: true, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}}, + {"strict credentials", []string{"-require-credential-hiding"}, governed.ShellPolicy{DataDir: "/d", Limits: capped, AllowNetworkWhereUndeniable: true}}, + {"strict network", []string{"-require-network-denial"}, governed.ShellPolicy{DataDir: "/d", Limits: capped, AllowCredentialReadsWhereUnhidable: true}}, { "limits", []string{"-max-memory", "512", "-max-processes", "64"}, governed.ShellPolicy{DataDir: "/d", Limits: sandbox.ResourceLimits{MemoryMiB: 512, MaxProcesses: 64}, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}, }, + { + "process cap keeps the default memory cap", + []string{"-max-processes", "64"}, + governed.ShellPolicy{DataDir: "/d", Limits: sandbox.ResourceLimits{MemoryMiB: 4096, MaxProcesses: 64}, AllowCredentialReadsWhereUnhidable: true, AllowNetworkWhereUndeniable: true}, + }, } { t.Run(tc.name, func(t *testing.T) { fs := flag.NewFlagSet("flynn", flag.ContinueOnError) diff --git a/docs/THREAT_MODEL.md b/docs/THREAT_MODEL.md index d30e6058..3bda23e5 100644 --- a/docs/THREAT_MODEL.md +++ b/docs/THREAT_MODEL.md @@ -220,7 +220,10 @@ it runs (`sandbox.Trust`, enforced at the dispatch boundary): both the single-conversation and fan-out paths. A per-command wall-clock cap is available at the sandbox boundary. Memory and process-count caps on a command the agent runs (`--max-memory`, `--max-processes`) are enforced where the platform supports it, through - the confined command's job object on Windows today; the Linux and macOS floors record the + the confined command's job object on Windows today. With no `--max-memory` the cap is half + the machine's physical memory, because the process cap alone does not bound memory: a few + hundred heavy processes can commit all of RAM long before the process limit is reached. + The Linux and macOS floors record the limits but do not yet impose them (a per-run cgroup needs cgroup-v2 delegation that is not guaranteed for an unprivileged agent), so a command needing a hard memory or process cap there runs under the container or hardware-isolation tier (see Coverage). diff --git a/docs/releases/v0.2.0.md b/docs/releases/v0.2.0.md index 404019ef..34c7e906 100644 --- a/docs/releases/v0.2.0.md +++ b/docs/releases/v0.2.0.md @@ -68,6 +68,7 @@ Run `flynn version` afterwards; it should name a `v0.2.0-rc` release. v0.1.3 can - Stored timestamps sort in time order. - Two Flynn processes opening the same new database at once (a goal starting while you run `flynn runs`, or two commands after an upgrade that adds migrations) could fail with `no such column: "uses"`, because both applied every migration. Each migration is now applied under the database write lock after checking it was not already applied, and the first connection waits out a competing one. - `go test`, `npm install` and `pip install` failed inside the sandbox: a confined command could not write the caches these tools keep under the home directory, on macOS it could not write its own `TMPDIR`, and on Windows its `TEMP` was the workspace, where Go ignores a `go.mod`. Each sandbox now has a private scratch directory outside the workspace, writable by its commands and deleted when the sandbox closes. The temp variables, `GOCACHE`, `GOMODCACHE`, `npm_config_cache` and `PIP_CACHE_DIR` point into it unless you set them yourself. On Linux it is under `/var/tmp`, so the caches last from one command to the next. +- On Windows a command the agent ran had no memory cap unless you passed `--max-memory`, and its process cap (4096) did not bound memory. A command that kept starting copies of itself, here a `go` shim on `PATH` that resolved back to itself, used all of a 95 GB machine's memory at a few hundred processes. With no `--max-memory`, a command and everything it starts may now commit at most half the machine's physical memory, in `flynn goal`, the interactive session, `flynn serve` and the MCP server. Pass `--max-memory` to set another cap. Local model servers are not affected. - A confined signer on Windows can read its sealed key. - The interactive session could hang reading input when a multi-byte character arrived with no room left for it in the read buffer. - Markdown in the terminal could render a line one cell wider than the screen, so it wrapped. diff --git a/internal/hardware/hardware.go b/internal/hardware/hardware.go index 40e12058..bc9b412e 100644 --- a/internal/hardware/hardware.go +++ b/internal/hardware/hardware.go @@ -108,6 +108,11 @@ func Detect(ctx context.Context) Box { return b } +// RAMBytes reports the machine's total physical memory in bytes, or 0 where it cannot be +// read. It is the RAM probe of [Detect] on its own, for a caller that sizes something +// against the host on every run and should not pay for the GPU and container probes. +func RAMBytes(ctx context.Context) int64 { return systemRAMBytes(ctx) } + // runCommand runs a probe tool and returns its standard output. It is a package-level // variable so the probe can be pointed at recorded tool output instead of the host, which // is how the decision logic above it is tested on a machine that has none of these tools.