From a8fbed656a3c13422a7f14bdc057bee6eabd670e Mon Sep 17 00:00:00 2001 From: Ion Alpha Date: Tue, 3 Nov 2026 08:36:59 +0000 Subject: [PATCH] fix(cmd/flynn): cap a command's memory at half of RAM by default On Windows a command the agent ran had no memory cap unless --max-memory was passed. The job object's process cap (4096) does not bound memory: a go shim on PATH that resolved back to itself started a few hundred heavy processes under a goal and committed all 95 GB of the machine, crashing WSL, long before reaching the process limit. With --max-memory unset, the agent's shell policy now caps each command at half the machine's physical memory, for flynn goal, the interactive session, flynn serve and the MCP server. An explicit --max-memory still wins, and where the RAM cannot be read the command runs uncapped as before. The default is set on the shell policy and not inside sandbox.Local, because the sandbox also runs local model servers, which can legitimately need more than half the machine. internal/hardware exports RAMBytes so the policy reads RAM without the GPU probes. A source test requires every governed.ShellPolicy built in cmd/flynn to set Limits, so a new entry point cannot start an uncapped shell. Signed-off-by: Ion Alpha --- cmd/flynn/mcp.go | 5 +- cmd/flynn/shellpolicy.go | 29 +++++++- cmd/flynn/shellpolicy_limits_test.go | 106 +++++++++++++++++++++++++++ cmd/flynn/shellpolicy_test.go | 17 +++-- docs/THREAT_MODEL.md | 5 +- docs/releases/v0.2.0.md | 1 + internal/hardware/hardware.go | 5 ++ 7 files changed, 159 insertions(+), 9 deletions(-) create mode 100644 cmd/flynn/shellpolicy_limits_test.go 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.