ReviewOS

also looking at this

reviewos/reviewos.org

fix: allow runtime arenas within a finite runner address budget

#7
Merged chris wants to merge codex/runner-virtual-address-budget into main
13 files +298 -12
.env.examplemodified+11-1
Changes to .env.example
@@ -442,7 +442,17 @@ TYPESENSE_API_KEY=pantry-dev
442442# these stop is the ordinary accident: a loop that writes a forty-gigabyte
443443# file, a build that spins a core forever. Generous on purpose, because a limit
444444# that trips on ordinary work is one an operator removes.
445# CI_STEP_MEMORY_MB=8192
445# Despite the legacy name, this ceiling is virtual address space in MiB
446# (`RLIMIT_AS`), not RSS or physical RAM. The finite default is 2 TiB.
447# Chrome's sandbox reserves about 1 TiB plus guard regions; the former 8 GiB
448# default stopped startup. Reserving addresses does not allocate that much RAM.
449# See [V8's sandbox documentation](https://v8.dev/blog/sandbox).
450#
451# Explicit values remain honored, including smaller caps and 0 to disable this
452# ceiling. An existing explicit 8 GiB cap can still prevent Chrome from starting.
453# macOS ignores this address-space limit. Resident-memory accounting and isolation
454# require separate OS controls; this setting does not provide them.
455# CI_STEP_MEMORY_MB=2097152
446456# CI_STEP_FILE_SIZE_MB=32768
447457#
448458# Off by default and worth understanding before setting: both count something
.github/scripts/record-runner-artifact.tsadded+71-0
Changes to .github/scripts/record-runner-artifact.ts
@@ -0,0 +1,71 @@
1import assert from 'node:assert/strict'
2import { readdir } from 'node:fs/promises'
3import { join } from 'node:path'
4
5// A normal Linux CI producer, not a local cross-build or an environment dump.
6assert.equal(process.platform, 'linux')
7assert.equal(process.arch, 'x64')
8assert.equal(process.env.GITHUB_ACTIONS, 'true')
9
10function git(...args: string[]): string {
11 const result = Bun.spawnSync(['git', ...args], { stdout: 'pipe', stderr: 'pipe' })
12 assert.equal(result.exitCode, 0, 'Cannot bind the runner artifact to its checkout')
13 return result.stdout.toString().trim()
14}
15
16async function sha256(path: string): Promise<string> {
17 return new Bun.CryptoHasher('sha256').update(await Bun.file(path).arrayBuffer()).digest('hex')
18}
19
20const source = git('rev-parse', 'HEAD')
21assert.match(source, /^[a-f0-9]{40}$/)
22assert.equal(source, process.env.GITHUB_SHA)
23assert.equal(git('status', '--porcelain', '--untracked-files=no'), '', 'The producer checkout changed during the build')
24
25const directory = 'dist/runner/linux-x64'
26assert.deepEqual(await readdir(directory), ['reviewos-runner'], 'Upload only the intended binary and generated public provenance')
27const binary = join(directory, 'reviewos-runner')
28const header = new DataView(await Bun.file(binary).slice(0, 64).arrayBuffer())
29assert.equal(header.byteLength, 64)
30assert.equal(header.getUint32(0, false), 0x7F454C46)
31assert.equal(header.getUint8(4), 2)
32assert.equal(header.getUint8(5), 1)
33assert.equal(header.getUint16(18, true), 62)
34
35const graph: Record<string, string> = {}
36for (const name of ['stacks', '@stacksjs/buddy', '@stacksjs/storage', '@stacksjs/validation', '@stacksjs/cli', '@stacksjs/types', '@stacksjs/stx', '@stacksjs/ts-cloud']) {
37 const metadata = await Bun.file(join('node_modules', name, 'package.json')).json()
38 assert.equal(metadata.name, name)
39 assert.equal(typeof metadata.version, 'string')
40 graph[name] = metadata.version
41}
42
43const buildCommand = await Bun.file('app/Commands/BuildRunner.ts').text()
44assert(buildCommand.includes('--no-compile-autoload-dotenv'))
45assert(buildCommand.includes('--no-compile-autoload-bunfig'))
46const binarySha256 = await sha256(binary)
47const proof = {
48 schema_version: 1,
49 source,
50 source_tree: git('rev-parse', 'HEAD^{tree}'),
51 workflow_run_id: process.env.GITHUB_RUN_ID,
52 workflow_run_attempt: process.env.GITHUB_RUN_ATTEMPT,
53 event: process.env.GITHUB_EVENT_NAME,
54 ref: process.env.GITHUB_REF,
55 bun: Bun.version,
56 target: 'bun-linux-x64',
57 installed_versions: graph,
58 inputs: {
59 package_json: await sha256('package.json'),
60 bun_lock: await sha256('bun.lock'),
61 entrypoint: await sha256('app/Actions/Runner/standalone.ts'),
62 limits: await sha256('app/Actions/Runner/limits.ts'),
63 build_command: await sha256('app/Commands/BuildRunner.ts'),
64 workflow: await sha256('.github/workflows/ci.yml'),
65 },
66 binary: { name: 'reviewos-runner', bytes: Bun.file(binary).size, sha256: binarySha256, format: 'ELF64', architecture: 'x64' },
67 compile_autoload: { dotenv: false, bunfig: false },
68}
69await Bun.write(join(directory, 'provenance.json'), `${JSON.stringify(proof, null, 2)}\n`)
70await Bun.write(join(directory, 'checksums.sha256'), `${binarySha256} reviewos-runner\n`)
71console.log(`Recorded Linux runner ${binarySha256} from ${source}`)
.github/workflows/ci.ymlmodified+50-2
Changes to .github/workflows/ci.yml
@@ -635,7 +635,7 @@ jobs:
635635 echo 'yes=true' >> "$GITHUB_OUTPUT"
636636 else
637637 echo 'yes=false' >> "$GITHUB_OUTPUT"
638 echo '::notice::compile: this app resolves the framework from npm and has no vendored storage/framework/core; nothing to compile.'
638 echo '::notice::compile: no vendored framework; skip Buddy compilation. The standalone runner is still built.'
639639 fi
640640
641641 - name: Setup Pantry
@@ -669,6 +669,22 @@ jobs:
669669 - name: Install Dependencies
670670 run: bun install --frozen-lockfile
671671
672 # The application runner has its own entrypoint even when the framework
673 # comes from npm. Preserve an original Linux CI artifact for fleet rollout.
674 - name: Compile the standalone Linux runner
675 run: ./buddy build:runner --target linux-x64 --out dist/runner/linux-x64
676
677 - name: Record standalone runner provenance
678 run: bun --no-env-file .github/scripts/record-runner-artifact.ts
679
680 - name: Preserve the standalone Linux runner and its provenance
681 uses: actions/upload-artifact@v4
682 with:
683 name: reviewos-runner-linux-x64-${{ github.sha }}
684 path: dist/runner/linux-x64/
685 if-no-files-found: error
686 retention-days: 14
687
672688 # The release job resolves through `pantry/`, not `node_modules`, and the
673689 # two lockfiles pin different versions — the first attempt at this check
674690 # passed here while the release still failed on a package `pantry.lock`
@@ -985,6 +1001,35 @@ jobs:
9851001 runs-on: ubuntu-latest
9861002 timeout-minutes: 15
9871003
1004 # The normal app build renders database-backed server blocks. Give those
1005 # pages the same disposable Postgres schema as the test suite, rather than
1006 # silently weakening server-script failures or reading production data.
1007 services:
1008 postgres:
1009 image: postgres:17
1010 env:
1011 POSTGRES_USER: postgres
1012 POSTGRES_PASSWORD: postgres
1013 POSTGRES_DB: reviewos
1014 ports:
1015 - 5432:5432
1016 options: >-
1017 --health-cmd pg_isready
1018 --health-interval 10s
1019 --health-timeout 5s
1020 --health-retries 5
1021
1022 env:
1023 DB_CONNECTION: postgres
1024 DB_HOST: 127.0.0.1
1025 DB_PORT: '5432'
1026 DB_DATABASE: reviewos
1027 DB_USERNAME: postgres
1028 DB_PASSWORD: postgres
1029 APP_URL: https://reviewos.test
1030 STACKS_DEFAULT_ROUTES: none
1031 APP_KEY: base64:Zm9yLWNpLW9ubHktbm90LWEtcmVhbC1hcHAta2V5LXg9
1032
9881033 steps:
9891034 - uses: actions/checkout@v6
9901035
@@ -1019,8 +1064,11 @@ jobs:
10191064 - name: Install Dependencies
10201065 run: bun install --frozen-lockfile
10211066
1067 - name: Migrate the preview database
1068 run: DB_MIGRATIONS_PATH="$PWD/database/migrations" ./buddy migrate
1069
10221070 - name: Build
10231071 run: bun run build
10241072
10251073 - name: Publish Commit
1026 run: ./storage/framework/scripts/publish-commit
1074 run: bash ./storage/framework/scripts/publish-commit
app/Actions/Feed/read.tsmodified+1-0
Changes to app/Actions/Feed/read.ts
@@ -1,3 +1,4 @@
1import { db } from '@stacksjs/database'
12import { describeEntry } from '../../Activity/verbs'
23
34/**
app/Actions/Runner/limits.tsmodified+4-3
Changes to app/Actions/Runner/limits.ts
@@ -64,9 +64,10 @@ export interface StepLimits {
6464 */
6565export function limitsFrom(env: Record<string, string | undefined> = process.env): StepLimits {
6666 return {
67 // Bun's production renderer reserves multi-GiB virtual arenas beyond its
68 // resident heap. A 4 GiB address ceiling can reject ordinary CSS rendering.
69 memoryMb: positive(env.CI_STEP_MEMORY_MB, 8192),
67 // Chrome reserves a roughly 1 TiB sandbox plus guard regions. Keep a finite
68 // 2 TiB virtual-address budget; an 8 GiB ceiling crashes ordinary startup.
69 // This is not an RSS/RAM limit. Explicit operator caps still take precedence.
70 memoryMb: positive(env.CI_STEP_MEMORY_MB, 2_097_152),
7071 // Off by default: see the note on the field. A per-user ceiling applied to
7172 // a build counts everything else the user is doing.
7273 processes: positive(env.CI_STEP_PROCESSES, 0),
docs/ci-security-review.mdmodified+6-0
Changes to docs/ci-security-review.md
@@ -102,6 +102,12 @@ writes a forty-gigabyte file, and it does nothing whatsoever about an attacker.
102102off by default because they count something wider than one step - `RLIMIT_NPROC` is per user, and
103103CPU seconds are not wall time.
104104
105The address-space default is a finite 2 TiB so modern runtime sandboxes can reserve
106their virtual arenas. This is `RLIMIT_AS`, not resident-memory accounting: it does
107not restrict a job to 2 TiB of physical RAM, reserve that much RAM for it, or provide
108a cgroup memory boundary. Explicit operator caps still apply unchanged. File,
109process and CPU ceilings are separate and retain their existing settings.
110
105111**What does not exist on the host path**, and must not be read as existing:
106112
107113- No memory limit that holds on macOS: `-v` is accepted there and ignored. No disk *quota* - the
docs/configuration.mdmodified+11-1
Changes to docs/configuration.md
@@ -1006,7 +1006,7 @@ can never leave is not a ceiling.
10061006
10071007### `CI_STEP_MEMORY_MB`
10081008
1009Default: `8192`, and the line is commented out.
1009Default: `2097152`, and the line is commented out.
10101010
10111011Ceilings a step cannot raise, applied before its first instruction.
10121012
@@ -1015,6 +1015,16 @@ sandbox that would change that is gated by docs/ci-security-review.md. What
10151015these stop is the ordinary accident: a loop that writes a forty-gigabyte
10161016file, a build that spins a core forever. Generous on purpose, because a limit
10171017that trips on ordinary work is one an operator removes.
1018Despite the legacy name, this ceiling is virtual address space in MiB
1019(`RLIMIT_AS`), not RSS or physical RAM. The finite default is 2 TiB.
1020Chrome's sandbox reserves about 1 TiB plus guard regions; the former 8 GiB
1021default stopped startup. Reserving addresses does not allocate that much RAM.
1022See [V8's sandbox documentation](https://v8.dev/blog/sandbox).
1023
1024Explicit values remain honored, including smaller caps and 0 to disable this
1025ceiling. An existing explicit 8 GiB cap can still prevent Chrome from starting.
1026macOS ignores this address-space limit. Resident-memory accounting and isolation
1027require separate OS controls; this setting does not provide them.
10181028
10191029Read by `app/Actions/Runner/limits.ts`.
10201030
docs/todo/06-search-explore.mdmodified+4-0
Changes to docs/todo/06-search-explore.md
@@ -71,6 +71,10 @@ because that is what actually listens.
7171 hide.
7272- [x] A Discover feed of public activity, with trending and recently active repositories beside it
7373
74 The shared feed reader imports its database binding explicitly, so static rendering does not
75 depend on application preloader globals. A fresh-process regression uses a local adapter
76 fixture without a database service; normal preview CI also renders against migrated Postgres.
77
7478 The event's recorded visibility and the repository's current visibility both
7579 matter. The first stops a repository made public today from exposing events
7680 written while it was private. The second stops a repository made private
docs/todo/09-checks-ci.mdmodified+13-3
Changes to docs/todo/09-checks-ci.md
@@ -1512,9 +1512,9 @@ gate, in order.
15121512- [x] A definitive output-delivery refusal revokes the host runner's lease immediately, so a
15131513 cancellation arriving through logs is acknowledged as cancelled before the next heartbeat.
15141514 Transient proxy failures retain their bounded output retries.
1515- [x] The default address-space ceiling allows Bun production rendering's virtual reservations,
1516 with an 8 GiB finite ceiling and explicit operator overrides retained. Verified with the
1517 unchanged production bundle and full marketing-page suite after 4 GiB CSS allocation failures.
1515- [x] Bun production rendering's virtual reservations no longer hit the original 4 GiB ceiling.
1516 The initial 8 GiB fix retained explicit operator overrides; the runtime sandbox budget below
1517 supersedes that default to support Chrome too.
15181518- [x] Host checkout retains only the job's lease-bound, read-only credential in an origin-scoped
15191519 Git helper, so later origin reads and production freshness checks work for private repositories.
15201520 Clone askpass files are removed immediately; operator credentials never enter the checkout and
@@ -1574,6 +1574,8 @@ gate, in order.
15741574
15751575 The mode it protects now runs real jobs - source, actions and secrets all reach the guest - so
15761576 this is a policy in front of something rather than in front of nothing.
1577- [x] Supply pull-request preview builds with a migrated, disposable Postgres database so database-backed server blocks build under the normal strict failure checks; invoke the tracked publication script through Bash.
1578
15771579- [x] Permit native build files under a bounded 32 GiB default: Android creates a 6 GiB device image and Xcode maps a 24 GiB sparse compilation cache. Real sparse-file regressions exercise the inherited kernel limit on macOS and Linux, including their different shell block units; explicit smaller operator limits remain enforced.
15781580
15791581- [x] CPU, memory, process, disk, output, and wall-time limits enforced outside the job
@@ -2455,3 +2457,11 @@ failed evidence into success.
24552457 policy-only cloud renderer. This prepares its sandbox configuration without provisioning,
24562458 resizing or restarting the runner; host application and browser acceptance are separate
24572459 operational checks.
2460
2461- [x] Allow runtime sandbox reservations within a finite 2 TiB default virtual-address budget,
2462 retaining explicit operator caps and the existing file, process and CPU ceilings. Linux
2463 kernel regressions exercise both generated command paths, release a large address-only
2464 arena, reject reservations above the cap and reject attempts to raise the hard cap.
2465 Normal Linux CI also produces the standalone runner with binary, source and installed-version
2466 provenance for a separately verified rollout. Resident-memory accounting remains a separate
2467 OS responsibility.
docs/todo/index.mdmodified+1-1
Changes to docs/todo/index.md
@@ -23,7 +23,7 @@ grows, so a phase getting *longer* while it is worked on is normal and honest.
2323| [06 - Search and explore](./06-search-explore.md) | Indexing, search, discovery | Complete (21/21) |
2424| [07 - Marketing and docs](./07-marketing-docs.md) | Landing page, documentation, self-hosting guide | Complete (59/59) |
2525| [08 - Migration](./08-migration.md) | Importing from GitHub and other forges | Done (16/16) |
26| [09 - Checks and CI](./09-checks-ci.md) | Checks, durable execution, runner providers, deployments | Started (132/133) |
26| [09 - Checks and CI](./09-checks-ci.md) | Checks, durable execution, runner providers, deployments | Started (134/135) |
2727| [10 - Federation](./10-federation.md) | AT Protocol identity: resolution, linking and OAuth sign-in | Complete (17/17) |
2828| [11 - Self-hosting and operations](./11-self-hosting-deploy.md) | Deployment, backups, upgrades, ops | Done (56/56) |
2929| [12 - The API and agents](./12-api-and-agents.md) | API parity, machine accounts, MCP, the CLI | Complete (30/30) |
tests/unit/feed-database-binding.test.tsadded+44-0
Changes to tests/unit/feed-database-binding.test.ts
@@ -0,0 +1,44 @@
1import { expect, test } from 'bun:test'
2import { fileURLToPath } from 'node:url'
3
4// The app preloader supplies db to ordinary requests. SSG imports the same
5// reader without that preloader, so test its dependency in a fresh process.
6// The adapter fixture performs no real database access or service startup.
7test('the feed reader owns its database binding without application globals', async () => {
8 const program = `
9 const assert = require('node:assert/strict')
10 const { mock } = require('bun:test')
11 assert.equal(typeof globalThis.db, 'undefined')
12 let reads = 0
13 const query = {
14 innerJoin() { return this },
15 select() { return this },
16 where() { return this },
17 orderBy() { return this },
18 limit() { return this },
19 async execute() { return [] },
20 }
21 mock.module('@stacksjs/database', () => ({
22 db: { selectFrom() { reads++; return query } },
23 }))
24 const { discoverFeed } = await import('./app/Actions/Feed/read.ts')
25 assert.deepEqual(await discoverFeed(), { entries: [], cursor: null })
26 assert.equal(reads, 1)
27 assert.equal(typeof globalThis.db, 'undefined')
28 console.log('feed database dependency resolved without globals')
29 `
30 const child = Bun.spawn([process.execPath, '--no-env-file', '--config=/dev/null', '-e', program], {
31 cwd: fileURLToPath(new URL('../..', import.meta.url)),
32 env: { PATH: process.env.PATH },
33 stdout: 'pipe',
34 stderr: 'pipe',
35 signal: AbortSignal.timeout(5_000),
36 })
37 const [output, error, code] = await Promise.all([
38 new Response(child.stdout).text(),
39 new Response(child.stderr).text(),
40 child.exited,
41 ])
42 expect({ code, error }).toEqual({ code: 0, error: '' })
43 expect(output.trim()).toBe('feed database dependency resolved without globals')
44}, 10_000)
tests/unit/runner-address-space.test.tsadded+75-0
Changes to tests/unit/runner-address-space.test.ts
@@ -0,0 +1,75 @@
1import { afterEach, describe, expect, test } from 'bun:test'
2import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
3import { tmpdir } from 'node:os'
4import { join } from 'node:path'
5import { limitedArgv, limitedCommand, limitsFrom } from '../../app/Actions/Runner/limits'
6
7const fixtures: string[] = []
8afterEach(() => fixtures.splice(0).forEach(path => rmSync(path, { recursive: true, force: true })))
9
10// Exercise the generated step boundary and the Linux kernel, not just the
11// configured number. PROT_NONE mappings reserve addresses without touching
12// their pages. This does not test resident-memory accounting or browser startup.
13describe.skipIf(process.platform !== 'linux')('the finite default virtual-address budget', () => {
14 test.each(['argv', 'command'])('%s permits runtime arenas while enforcing the inherited hard cap', async (mode) => {
15 const root = mkdtempSync(join(tmpdir(), 'reviewos-address-space-'))
16 fixtures.push(root)
17 const program = join(root, 'reserve.js')
18 writeFileSync(program, `
19 const assert = require('node:assert/strict')
20 const { dlopen, read } = require('bun:ffi')
21 const libc = dlopen('libc.so.6', {
22 getrlimit: { args: ['i32', 'ptr'], returns: 'i32' },
23 setrlimit: { args: ['i32', 'ptr'], returns: 'i32' },
24 mmap: { args: ['ptr', 'u64', 'i32', 'i32', 'i32', 'i64'], returns: 'u64' },
25 munmap: { args: ['u64', 'u64'], returns: 'i32' },
26 __errno_location: { args: [], returns: 'u64' },
27 })
28 try {
29 const failed = 18446744073709551615n
30 const arenaBytes = 1168231104512n // 1 TiB plus two 32 GiB guard regions.
31 const arena = libc.symbols.mmap(null, arenaBytes, 0, 0x4022, -1, 0)
32 assert.notEqual(arena, failed, 'The default must permit the runtime virtual arena')
33 assert.equal(libc.symbols.munmap(arena, arenaBytes), 0)
34 const before = Buffer.alloc(16)
35 assert.equal(libc.symbols.getrlimit(9, before), 0)
36 assert.equal(before.readBigUInt64LE(0), 2199023255552n)
37 assert.equal(before.readBigUInt64LE(8), 2199023255552n)
38 const oversizedBytes = 3298534883328n // 3 TiB, above the finite default.
39 const oversized = libc.symbols.mmap(null, oversizedBytes, 0, 0x4022, -1, 0)
40 const allocationError = read.i32(libc.symbols.__errno_location())
41 if (oversized !== failed) libc.symbols.munmap(oversized, oversizedBytes)
42 assert.equal(oversized, failed)
43 assert.equal(allocationError, 12) // ENOMEM.
44 const raised = Buffer.alloc(16)
45 raised.writeBigUInt64LE(2199023259648n, 0)
46 raised.writeBigUInt64LE(2199023259648n, 8)
47 assert.equal(libc.symbols.setrlimit(9, raised), -1)
48 assert.equal(read.i32(libc.symbols.__errno_location()), 1) // EPERM.
49 const after = Buffer.alloc(16)
50 assert.equal(libc.symbols.getrlimit(9, after), 0)
51 assert.deepEqual(after, before)
52 console.log(JSON.stringify({ arenaReleased: true, oversizedRejected: true, hardCapRetained: true, pagesTouched: false }))
53 }
54 finally { libc.close() }
55 `)
56 const args = [process.execPath, '--no-env-file', program]
57 const limits = limitsFrom({})
58 const command = mode === 'argv'
59 ? limitedArgv(args, limits)
60 : ['/bin/sh', '-c', limitedCommand('exec "$@"', limits), 'test-step', ...args]
61 const child = Bun.spawn(command, {
62 cwd: root,
63 stdout: 'pipe',
64 stderr: 'pipe',
65 signal: AbortSignal.timeout(10_000),
66 })
67 const [output, error, code] = await Promise.all([
68 new Response(child.stdout).text(),
69 new Response(child.stderr).text(),
70 child.exited,
71 ])
72 expect({ code, error }).toEqual({ code: 0, error: '' })
73 expect(JSON.parse(output)).toEqual({ arenaReleased: true, oversizedRejected: true, hardCapRetained: true, pagesTouched: false })
74 }, 15_000)
75})
tests/unit/runner-limits.test.tsmodified+7-1
Changes to tests/unit/runner-limits.test.ts
@@ -29,10 +29,16 @@ describe('the settings', () => {
2929 test('are generous by default, because a limit that trips on ordinary work is one an operator removes', () => {
3030 const limits = limitsFrom({})
3131
32 expect(limits.memoryMb).toBe(8192)
32 expect(limits.memoryMb).toBe(2_097_152)
3333 expect(limits.fileSizeMb).toBe(32768)
3434 })
3535
36 test('keeps explicit virtual-address caps, including an explicit opt-out', () => {
37 expect(limitsFrom({ CI_STEP_MEMORY_MB: '8192' }).memoryMb).toBe(8192)
38 expect(limitsFrom({ CI_STEP_MEMORY_MB: '2048' }).memoryMb).toBe(2048)
39 expect(limitsFrom({ CI_STEP_MEMORY_MB: '0' }).memoryMb).toBe(0)
40 })
41
3642 test('and the process ceiling is off, because it counts the whole user rather than the step', () => {
3743 /*
3844 * `RLIMIT_NPROC` is per real UID. On a machine where the runner shares a