From 26336b8ce84835091214e047a448a31e9c42266f Mon Sep 17 00:00:00 2001 From: Long Ho Date: Mon, 14 Sep 2026 01:32:39 +0000 Subject: [PATCH 1/2] feat: run VRT with relocatable declared runtimes --- .github/workflows/actiond-production.yaml | 1 - docs/actiond.md | 20 +++-- docs/browser-runtime.md | 28 +++++-- experiments/actiond/README.md | 13 ++- experiments/actiond/isolation.visual.spec.ts | 3 + internal/remote.bzl | 9 +- playwright/defs.bzl | 4 + runtime/BUILD.bazel | 10 +++ runtime/relocation.test.ts | 68 +++++++++++++++ runtime/relocation.ts | 87 ++++++++++++++++++++ runtime/remote-job.ts | 51 ++++++++++++ runtime/remote-result.test.ts | 15 +--- runtime/remote-runner.ts | 52 ++---------- runtime/runner.ts | 9 ++ runtime/vrt-processes.cjs | 17 ++++ 15 files changed, 309 insertions(+), 78 deletions(-) create mode 100644 runtime/relocation.test.ts create mode 100644 runtime/relocation.ts create mode 100644 runtime/remote-job.ts create mode 100644 runtime/vrt-processes.cjs diff --git a/.github/workflows/actiond-production.yaml b/.github/workflows/actiond-production.yaml index 7cb3dd8..552a636 100644 --- a/.github/workflows/actiond-production.yaml +++ b/.github/workflows/actiond-production.yaml @@ -45,7 +45,6 @@ jobs: run: | work="$RUNNER_TEMP/actiond-production" git -C "$work/actiond" apply "$GITHUB_WORKSPACE/experiments/actiond/actiond-advice.patch" - git -C "$work/actiond" apply "$GITHUB_WORKSPACE/experiments/actiond/actiond-input-rootfs.patch" cd "$work/actiond" bazelisk --output_base="$work/worker-output" build --bes_backend= --remote_executor= --remote_cache= --spawn_strategy=local --jobs=2 //cmd/linux-actiond:linux-actiond_linux_x86_64 > "$work/worker-build.log" 2>&1 worker=$(bazelisk --output_base="$work/worker-output" cquery --bes_backend= //cmd/linux-actiond:linux-actiond_linux_x86_64 --output=starlark '--starlark:expr=providers(target)["DefaultInfo"].files_to_run.executable.path') diff --git a/docs/actiond.md b/docs/actiond.md index b511889..f3e71ec 100644 --- a/docs/actiond.md +++ b/docs/actiond.md @@ -7,11 +7,12 @@ from their own OCI image. Host E2E and component tests use host browsers. ## Worker and Bazel configuration -The worker currently needs the memory-advice and declared-rootfs patches in -[`experiments/actiond`](../experiments/actiond). The memory-advice change is -[upstream PR #48](https://github.com/hermeticbuild/actiond/pull/48); the rootfs -patch is maintained locally for upstreaming. The production workflow builds and -runs that exact patched worker. Linux VM workers require KVM and vhost-vsock. +The worker currently needs the memory-advice kernel patch in +[`experiments/actiond`](../experiments/actiond), covered by +[upstream PR #33](https://github.com/hermeticbuild/actiond/pull/33). +The production workflow builds actiond with only that patch. No `input-rootfs`, +`libc`, or `requires-bash` execution properties are needed. +Linux VM workers require KVM and vhost-vsock. With a patched worker listening on `127.0.0.1:8980`, put this in the consumer's Bazel configuration: @@ -34,8 +35,7 @@ Use a remote worker address when appropriate. ARM64 clients need an amd64 worker for these baselines. The validated CI worker uses 6 GiB RAM for at most two concurrent actions; size workers for fixture and staging memory as well as Chromium. The macOS native VM backend is not yet validated by these -checks. Local fallback must remain disabled; `/workspace` runtime executables -are meaningful inside the worker's declared rootfs. +checks. Keep VRT's explicit remote strategies and local fallback disabled. ```sh bazel test --config=vrt //path:visual_test @@ -56,6 +56,12 @@ commands needed by fixtures. OCI extraction verifies declared blobs and never contacts a registry. Acquisition and image construction happen before execution; Docker, registry credentials, Testcontainers, and Ryuk are absent from the action. +The declared ELF loader starts the bootstrap. It prepares a private Node/Bash +launcher in the action's temporary directory, then relocates executable copies +in the staged inputs. Libraries and fonts use explicit paths. A VRT-only Node +preload directs `spawn(..., {shell: true})` to declared Bash, preserving native +Playwright `webServer` behavior without `/bin/sh`. Host tests do not load it. + The action has loopback-only networking. Start fixture services inside it using `server` or native Playwright `webServer`, and declare their files in `data`. External assets and APIs need local fixtures. Live deployed checks belong in diff --git a/docs/browser-runtime.md b/docs/browser-runtime.md index 580a0c6..20bfc36 100644 --- a/docs/browser-runtime.md +++ b/docs/browser-runtime.md @@ -2,10 +2,10 @@ The actiond migration accepts caller-owned Linux runtime files through `browser_runtime`. The runtime must contain Chromium, Node, their ELF loader and -shared libraries, and the fonts/fontconfig used for screenshots. Native -Playwright `webServer` commands also need `/bin/sh`. Bazel `js_binary` fixture -launchers additionally need `/usr/bin/env`, Bash, and their shell utilities -(including `dirname`, `uname`, and `readlink`) in the declared runtime. +shared libraries, and the fonts/fontconfig used for screenshots. Include Bash +and the shell utilities used by fixture launchers (including `dirname`, `uname`, +and `readlink` for Bazel `js_binary`). These files stay inside the runtime tree; +they are not installed at system paths. A caller can produce a flattened filesystem tar and unpack it during the Bazel build: @@ -25,6 +25,8 @@ browser_runtime( root = ":runtime_files", executable = "chromium/chrome-headless-shell", node = "bin/node", + loader = "lib/ld-linux-x86-64.so.2", + bash = "bin/bash", library_dirs = ["lib"], fontconfig = "etc/fonts", ) @@ -46,8 +48,22 @@ Archive extraction uses a Bazel-provided Python interpreter and makes no network requests. Absolute image symlinks are resolved within the image root, then links are materialized into regular files and directories for the output tree. Missing link targets are errors, so runtime packaging cannot silently borrow host files. -Font configuration should use image paths or paths relative to the configuration -file, rather than a build-machine path. +Font configuration must use paths relative to its configuration file, rather +than absolute image or build-machine paths. + +`loader` and `bash` default to the paths shown above. Execution starts the +declared loader directly. The VRT runner creates `/tmp/rules-web-vrt` inside its +isolated action, copies the loader/Node/Bash there, and rewrites staged ELF64 +interpreter paths to that loader. This retains executable identity for Chromium +subprocesses. Interpreter segments too short for the replacement are rejected. +Original declared inputs are never modified. + +Executable scripts with ordinary `/bin/sh`, `/bin/bash`, `/usr/bin/env bash`, +or Node shebangs are redirected to the declared launchers. The VRT subprocess +adapter supplies Bash for Playwright's `shell: true` launches. Other hardcoded +system paths, interpreters, and complex `env -S` shebangs need caller-owned +wrappers or packaging changes; arbitrary OCI images are not automatically +relocatable. For a caller-owned OCI image layout directory, use `browser_runtime_oci` instead: diff --git a/experiments/actiond/README.md b/experiments/actiond/README.md index bf10697..c2b4247 100644 --- a/experiments/actiond/README.md +++ b/experiments/actiond/README.md @@ -74,7 +74,7 @@ The local host has no `/dev/kvm`; KVM validation ran on GitHub's Ubuntu runner. ## Production validation -The production workflow builds actiond at `8a42c3d` with both patches below, +The production workflow builds actiond at `8a42c3d` with the memory-advice patch, starts a Linux amd64 VM, and runs `prepare-public.mjs` / `run-public-actiond.sh`. The fixture constructs a caller-owned OCI layout through Bazel and extracts its runtime with `browser_runtime_oci`. Public `.update` and test targets execute @@ -96,14 +96,21 @@ is separate from the VM fixtures and does not establish a full consumer CI migra ## Local actiond patches - `actiond-advice.patch`: enables memory-advice syscalls in both kernel configs; - submitted as [upstream PR #48](https://github.com/hermeticbuild/actiond/pull/48). -- `actiond-input-rootfs.patch`: exposes selected directories from a declared + covered by [upstream PR #33](https://github.com/hermeticbuild/actiond/pull/33). +- `actiond-input-rootfs.patch`: historical alternative, submitted as + [upstream PR #49](https://github.com/hermeticbuild/actiond/pull/49), and no longer + applied by production CI. It exposes selected directories from a declared runtime input tree at normal Linux paths, preserving executor-owned devices, `/proc`, temporary storage, and network isolation. `input-rootfs-env` resolves its path from a declared command variable, which supports Bazel output paths. Maintained separately in local actiond commits `66e2dca` and `f713bca` for upstreaming. The full actiond build and both unit-test targets pass. +The current VRT runner instead relocates staged executable interpreter paths, +supplies explicit library/font paths, and directs Node shell launches to declared +Bash. The production isolation fixture asserts that the former system runtime +paths are absent. Image extraction remains a caller-side Bazel action. + The native macOS VM backend has not been exercised here. ARM64 clients must select an amd64 worker for these baseline inputs. diff --git a/experiments/actiond/isolation.visual.spec.ts b/experiments/actiond/isolation.visual.spec.ts index 3ea2d56..bed1935 100644 --- a/experiments/actiond/isolation.visual.spec.ts +++ b/experiments/actiond/isolation.visual.spec.ts @@ -6,6 +6,9 @@ test('the whole VRT action is offline and can still serve its fixture', async ({ expect(process.platform).toBe('linux') expect(process.arch).toBe('x64') expect(fs.existsSync('/var/run/docker.sock')).toBe(false) + // No input-rootfs mapping or packaged libc/Bash runtime was requested. + for (const file of ['/bin/bash', '/bin/sh', '/usr/bin/env', '/lib64/ld-linux-x86-64.so.2']) + expect(fs.existsSync(file), file).toBe(false) const fixture = JSON.parse(process.env.ACTIOND_FIXTURE!) expect(JSON.parse(fs.readFileSync(fixture.package, 'utf8')).name).toBeTruthy() const error = await new Promise(resolve => { diff --git a/internal/remote.bzl b/internal/remote.bzl index 901c2b0..831bfaf 100644 --- a/internal/remote.bzl +++ b/internal/remote.bzl @@ -43,20 +43,20 @@ def _remote_impl(ctx): "args": [ctx.expand_location(value, targets = locations) for value in ctx.attr.args], "output": output.path, "capture": ctx.attr.capture, + "runtime": dict(browser.descriptor, path = root.path), })) descriptor = browser.descriptor ctx.actions.run( - executable = "/workspace/" + root.path + "/" + descriptor["node"], - arguments = [ctx.file._bootstrap.path, job.path], + executable = root.path + "/" + descriptor["loader"], + arguments = [root.path + "/" + descriptor["node"], ctx.file._bootstrap.path, job.path], inputs = depset(files + [root, job, ctx.file._bootstrap]), outputs = [output], env = { - "VRT_RUNTIME_ROOT": root.path, "HOME": "/tmp", "TMPDIR": "/tmp", "LANG": "C.UTF-8", "TZ": "UTC", - "LD_LIBRARY_PATH": ":".join(["/workspace/" + root.path + "/" + p for p in descriptor["libraryDirs"]]), + "LD_LIBRARY_PATH": ":".join([root.path + "/" + p for p in descriptor["libraryDirs"]]), }, execution_requirements = {"no-local": "1"}, mnemonic = "VrtCapture" if ctx.attr.capture else "VrtCompare", @@ -96,7 +96,6 @@ def remote_browser_test(name, browser, env, args, tags, timeout, data, target_pl capture = capture, data = data, target_platform = target_platform, - exec_properties = {"input-rootfs-env": "VRT_RUNTIME_ROOT"}, exec_compatible_with = [Label("@platforms//os:linux"), Label("@platforms//cpu:x86_64")], tags = ["manual"], ) diff --git a/playwright/defs.bzl b/playwright/defs.bzl index 2da5c8a..fae0818 100644 --- a/playwright/defs.bzl +++ b/playwright/defs.bzl @@ -18,6 +18,8 @@ def _browser_runtime_impl(ctx): "root": runfile(ctx.file.root), "executable": _relative_path(ctx.attr.executable, "executable"), "node": _relative_path(ctx.attr.node, "node"), + "loader": _relative_path(ctx.attr.loader, "loader"), + "bash": _relative_path(ctx.attr.bash, "bash"), "libraryDirs": [_relative_path(p, "library_dirs") for p in ctx.attr.library_dirs], "fontconfig": _relative_path(ctx.attr.fontconfig, "fontconfig"), "arch": ctx.attr.arch, @@ -34,6 +36,8 @@ browser_runtime = rule( "root": attr.label(mandatory = True, allow_single_file = True), "executable": attr.string(mandatory = True), "node": attr.string(mandatory = True), + "loader": attr.string(default = "lib/ld-linux-x86-64.so.2"), + "bash": attr.string(default = "bin/bash"), "library_dirs": attr.string_list(mandatory = True), "fontconfig": attr.string(mandatory = True), "arch": attr.string(default = "x64", values = ["x64", "arm64"]), diff --git a/runtime/BUILD.bazel b/runtime/BUILD.bazel index 7bccb51..dd0c0f4 100644 --- a/runtime/BUILD.bazel +++ b/runtime/BUILD.bazel @@ -46,6 +46,9 @@ js_library( "host-browser.js", "matching.js", "network.js", + "relocation.js", + "remote-job.js", + "vrt-processes.cjs", "package.json", "server.js", "static-server.js", @@ -87,6 +90,13 @@ js_test( entry_point = "remote-result.test.js", ) +js_test( + name = "relocation_test", + size = "small", + data = ["package.json", ":typecheck", "vrt-processes.cjs"], + entry_point = "relocation.test.js", +) + npm_package( name = "package", srcs = [ diff --git a/runtime/relocation.test.ts b/runtime/relocation.test.ts new file mode 100644 index 0000000..ef63923 --- /dev/null +++ b/runtime/relocation.test.ts @@ -0,0 +1,68 @@ +import assert from 'node:assert/strict' +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import {spawnSync} from 'node:child_process' +import {fileURLToPath} from 'node:url' +import {test} from 'node:test' +import {relocateExecutable, relocateInputs, runtimeDirectory} from './relocation.js' + +function elf(interpreter = '/lib64/ld-linux-x86-64.so.2') { + const data = Buffer.alloc(256, 0x51) + data.set([0x7f, 0x45, 0x4c, 0x46, 2, 1]) + data.writeBigUInt64LE(64n, 32) + data.writeUInt16LE(56, 54) + data.writeUInt16LE(1, 56) + data.writeUInt32LE(3, 64) + data.writeBigUInt64LE(128n, 72) + data.writeBigUInt64LE(BigInt(interpreter.length + 1), 96) + data.write(interpreter + '\0', 128) + return data +} + +test('ELF relocation replaces only the interpreter bytes and rejects invalid segments', () => { + const input = elf() + const output = relocateExecutable(input)! + assert.deepEqual(output.subarray(0, 128), input.subarray(0, 128)) + assert.deepEqual(output.subarray(156), input.subarray(156)) + assert.equal(output.subarray(128, 128 + runtimeDirectory.length + 7).toString(), `${runtimeDirectory}/ld.so\0`) + assert.equal(input.subarray(128, 156).toString(), '/lib64/ld-linux-x86-64.so.2\0') + assert.throws(() => relocateExecutable(elf('/ld.so')), /too short/) + const bad = elf(); bad.writeBigUInt64LE(999n, 72) + assert.throws(() => relocateExecutable(bad), /Invalid ELF interpreter/) + assert.throws(() => relocateExecutable(input.subarray(0, 100)), /program-header/) +}) + +test('staging relocates executable ELF and common shebangs without changing data or following links', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'vrt-relocation-')) + try { + const binary = path.join(root, 'node'); fs.writeFileSync(binary, elf(), {mode: 0o555}) + const script = path.join(root, 'server'); fs.writeFileSync(script, '#!/usr/bin/env bash\nprintf hello\n', {mode: 0o755}) + const data = path.join(root, 'data'); fs.writeFileSync(data, elf(), {mode: 0o644}) + fs.symlinkSync('data', path.join(root, 'alias')) + relocateInputs(root) + assert.deepEqual(fs.readFileSync(binary), relocateExecutable(elf())) + assert.equal(fs.statSync(binary).mode & 0o777, 0o555) + assert.equal(fs.readFileSync(script, 'utf8'), `#!${runtimeDirectory}/bash\nprintf hello\n`) + assert.deepEqual(fs.readFileSync(data), elf()) + assert.ok(fs.lstatSync(path.join(root, 'alias')).isSymbolicLink()) + assert.equal(relocateExecutable(Buffer.from('#!/usr/bin/env python3\nprint(1)\n')), undefined) + } finally { fs.rmSync(root, {recursive: true, force: true}) } +}) + +test('VRT subprocess adapter supplies a declared shell while preserving explicit shell selection', {skip: process.platform === 'win32'}, () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'vrt-shell-')) + try { + const shell = path.join(root, 'bash') + fs.writeFileSync(shell, '#!/bin/sh\nprintf "declared:"\nexec /bin/sh "$@"\n', {mode: 0o755}) + const hook = fileURLToPath(new URL('./vrt-processes.cjs', import.meta.url)) + const probe = `const {spawn}=require('node:child_process'); const c=spawn('printf hello',{shell:SHELL});c.stdout.pipe(process.stdout);c.on('exit',code=>process.exit(code));` + for (const [selection, expected] of [['true', 'declared:hello'], ['"/bin/sh"', 'hello']]) { + const result = spawnSync(process.execPath, ['--require', hook, '-e', probe.replace('SHELL', selection)], { + env: {...process.env, VRT_BASH: shell}, encoding: 'utf8', + }) + assert.equal(result.status, 0, result.stderr) + assert.equal(result.stdout, expected) + } + } finally { fs.rmSync(root, {recursive: true, force: true}) } +}) diff --git a/runtime/relocation.ts b/runtime/relocation.ts new file mode 100644 index 0000000..70552d2 --- /dev/null +++ b/runtime/relocation.ts @@ -0,0 +1,87 @@ +import fs from 'node:fs' +import path from 'node:path' + +// Each action owns its /tmp namespace. A short interpreter path fits existing +// ELF PT_INTERP segments without moving loadable segments or changing offsets. +export const runtimeDirectory = '/tmp/rules-web-vrt' +const interpreter = Buffer.from(`${runtimeDirectory}/ld.so\0`) + +/** Rewrite executable lookup paths, leaving ELF loadable segments untouched. */ +export function relocateExecutable(data: Buffer): Buffer | undefined { + if (data.subarray(0, 4).equals(Buffer.from([0x7f, 0x45, 0x4c, 0x46]))) { + if (data.length < 64 || data[4] !== 2 || data[5] !== 1) + throw new Error('VRT relocation requires a little-endian ELF64 executable') + const table = Number(data.readBigUInt64LE(32)) + const size = data.readUInt16LE(54) + const count = data.readUInt16LE(56) + if (!Number.isSafeInteger(table) || size < 56 || table < 64 || table + size * count > data.length) + throw new Error('Invalid ELF program-header table') + for (let index = 0; index < count; index++) { + const header = table + index * size + if (data.readUInt32LE(header) !== 3) continue + const offset = Number(data.readBigUInt64LE(header + 8)) + const length = Number(data.readBigUInt64LE(header + 32)) + if (!Number.isSafeInteger(offset) || !Number.isSafeInteger(length) || offset < 0 || offset + length > data.length) + throw new Error('Invalid ELF interpreter segment') + if (length < interpreter.length) + throw new Error('ELF interpreter segment is too short for VRT relocation') + const result = Buffer.from(data) + result.fill(0, offset, offset + length) + interpreter.copy(result, offset) + return result + } + return undefined + } + if (data.subarray(0, 2).toString() !== '#!') return undefined + const end = data.indexOf(10) + if (end < 0) return undefined + const line = data.subarray(2, end).toString().trim() + const match = /^(?:\/usr\/bin\/env\s+|\/(?:usr\/)?bin\/)(bash|sh|node)$/.exec(line) + if (!match) return undefined + const executable = match[1] === 'node' ? 'node' : 'bash' + return Buffer.concat([Buffer.from(`#!${runtimeDirectory}/${executable}\n`), data.subarray(end + 1)]) +} + +function relocateFile(file: string) { + const mode = fs.statSync(file).mode + if (!(mode & 0o111)) return + const data = fs.readFileSync(file) + const relocated = relocateExecutable(data) + if (!relocated) return + fs.chmodSync(file, mode | 0o200) + fs.writeFileSync(file, relocated) + fs.chmodSync(file, mode) +} + +/** Only staged copies are changed; declared CAS inputs remain immutable. */ +export function relocateInputs(directory: string): void { + for (const entry of fs.readdirSync(directory, {withFileTypes: true})) { + const file = path.join(directory, entry.name) + if (entry.isDirectory()) relocateInputs(file) + else if (entry.isFile()) relocateFile(file) + } +} + +export function bootstrapRuntime(runtime: { + path: string; loader: string; node: string; bash: string; libraryDirs: string[] +}) { + const root = fs.realpathSync(runtime.path) + const resolve = (relative: string) => { + if (!relative || path.isAbsolute(relative) || relative.split('/').some(p => !p || p === '.' || p === '..')) + throw new Error(`Invalid runtime path: ${relative}`) + const file = fs.realpathSync(path.join(root, relative)) + if (!file.startsWith(root + path.sep)) throw new Error(`Runtime path escapes declared root: ${relative}`) + return file + } + fs.mkdirSync(runtimeDirectory) + for (const [name, source] of [['ld.so', runtime.loader], ['node', runtime.node], ['bash', runtime.bash]]) { + const output = path.join(runtimeDirectory, name) + fs.copyFileSync(resolve(source), output) + fs.chmodSync(output, 0o755) + if (name !== 'ld.so') relocateFile(output) + } + return { + node: path.join(runtimeDirectory, 'node'), + libraryPath: runtime.libraryDirs.map(resolve).join(path.delimiter), + } +} diff --git a/runtime/remote-job.ts b/runtime/remote-job.ts new file mode 100644 index 0000000..6707c0a --- /dev/null +++ b/runtime/remote-job.ts @@ -0,0 +1,51 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import {spawnSync} from 'node:child_process' + +export interface RemoteJob { + runfiles: Record + runner: string + env: Record + args: string[] + output: string + capture: boolean +} + +/** Preserve child failure reports as build outputs for the local result consumer. */ +export function runRemoteJob(job: RemoteJob, node: string, environment: NodeJS.ProcessEnv = {}) { + const output = path.resolve(job.output) + const temp = fs.mkdtempSync(path.join(os.tmpdir(), 'vrt-action-')) + const artifacts = path.join(output, 'artifacts') + fs.mkdirSync(artifacts, {recursive: true}) + const escape = (value: string) => value.replaceAll('\\', '\\b').replaceAll(' ', '\\s').replaceAll('\n', '\\n') + const manifest = path.join(temp, 'MANIFEST') + fs.writeFileSync(manifest, Object.entries(job.runfiles).map(([name, source]) => + ` ${escape(name)} ${escape(path.resolve(source))}\n` + ).join('')) + const result = spawnSync(node, [ + path.resolve(job.runner), ...job.args, ...(job.capture ? ['--update'] : []), + ], { + env: { + ...process.env, + ...job.env, + ...environment, + RUNFILES_DIR: temp, + RUNFILES_MANIFEST_FILE: manifest, + JS_BINARY__NODE_BINARY: node, + TEST_TMPDIR: temp, + TEST_UNDECLARED_OUTPUTS_DIR: artifacts, + VRT_CAPTURE_OUTPUT: job.capture ? path.join(output, 'baselines') : '', + }, + stdio: 'inherit', + }) + if (result.error) console.error(result.error) + // Return a successful build action even when tests fail, so Bazel downloads + // their reports and screenshots. The local test wrapper returns this status. + fs.writeFileSync(path.join(output, 'result.json'), JSON.stringify({ + schemaVersion: 1, + mode: job.capture ? 'capture' : 'compare', + exitCode: result.status ?? 1, + })) + fs.rmSync(temp, {recursive: true, force: true}) +} diff --git a/runtime/remote-result.test.ts b/runtime/remote-result.test.ts index e19e73a..20756d5 100644 --- a/runtime/remote-result.test.ts +++ b/runtime/remote-result.test.ts @@ -3,9 +3,8 @@ import fs from 'node:fs' import os from 'node:os' import path from 'node:path' import {test} from 'node:test' -import {spawnSync} from 'node:child_process' -import {fileURLToPath} from 'node:url' import {consumeRemoteResult} from './remote-result.js' +import {runRemoteJob} from './remote-job.js' test('failed remote capture preserves local baselines and returns failure artifacts', t => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'vrt-result-')) @@ -53,7 +52,7 @@ test('result metadata and artifact links cannot bypass local validation', t => { assert.throws(() => consumeRemoteResult(root, {artifacts: path.join(root, 'downloads')}), /only regular files/) }) -test('remote bootstrap preserves a failing subprocess result for the local test', t => { +test('remote job preserves a failing subprocess result for the local test', t => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'vrt-bootstrap-')) t.after(() => fs.rmSync(root, {recursive: true, force: true})) const runner = path.join(root, 'consumer.mjs') @@ -61,16 +60,8 @@ test('remote bootstrap preserves a failing subprocess result for the local test' fs.writeFileSync(process.env.TEST_UNDECLARED_OUTPUTS_DIR + '/junit.xml', ''); process.exitCode = 7;`) const output = path.join(root, 'output') - const job = path.join(root, 'job.json') - fs.writeFileSync(job, JSON.stringify({ - runfiles: {}, runner, env: {}, args: [], output, capture: false, - })) - // The VM invokes Node directly, without rules_js's process.execPath wrapper. const node = fs.realpathSync(process.env.JS_BINARY__NODE_BINARY || process.execPath) - const result = spawnSync(node, [ - fileURLToPath(new URL('./remote-runner.js', import.meta.url)), job, - ], {encoding: 'utf8', env: {...process.env, NODE_OPTIONS: ''}, timeout: 5000}) - assert.equal(result.status, 0, result.stderr) + runRemoteJob({runfiles: {}, runner, env: {}, args: [], output, capture: false}, node, {NODE_OPTIONS: ''}) const artifacts = path.join(root, 'test-artifacts') assert.equal(consumeRemoteResult(output, {artifacts}), 7) assert.equal(fs.readFileSync(path.join(artifacts, 'junit.xml'), 'utf8'), '') diff --git a/runtime/remote-runner.ts b/runtime/remote-runner.ts index 142e579..14f70fd 100644 --- a/runtime/remote-runner.ts +++ b/runtime/remote-runner.ts @@ -1,47 +1,11 @@ -// Runs inside the Linux execution action. Result consumption stays local. +// The initial Node is invoked through the caller's ELF loader. Relaunch with a +// relocated Node so /proc/self/exe and child processes resolve the executable. import fs from 'node:fs' -import os from 'node:os' -import path from 'node:path' -import {spawnSync} from 'node:child_process' +import {bootstrapRuntime} from './relocation.js' +import {runRemoteJob, type RemoteJob} from './remote-job.js' -const job = JSON.parse(fs.readFileSync(process.argv[2], 'utf8')) as { - runfiles: Record - runner: string - env: Record - args: string[] - output: string - capture: boolean +const job = JSON.parse(fs.readFileSync(process.argv[2], 'utf8')) as RemoteJob & { + runtime: Parameters[0] } -const output = path.resolve(job.output) -const temp = fs.mkdtempSync(path.join(os.tmpdir(), 'vrt-action-')) -const artifacts = path.join(output, 'artifacts') -fs.mkdirSync(artifacts, {recursive: true}) -const escape = (value: string) => value.replaceAll('\\', '\\b').replaceAll(' ', '\\s').replaceAll('\n', '\\n') -const manifest = path.join(temp, 'MANIFEST') -fs.writeFileSync(manifest, Object.entries(job.runfiles).map(([name, source]) => - ` ${escape(name)} ${escape(path.resolve(source))}\n` -).join('')) -const result = spawnSync(process.execPath, [ - path.resolve(job.runner), ...job.args, ...(job.capture ? ['--update'] : []), -], { - env: { - ...process.env, - ...job.env, - RUNFILES_DIR: temp, - RUNFILES_MANIFEST_FILE: manifest, - JS_BINARY__NODE_BINARY: process.execPath, - TEST_TMPDIR: temp, - TEST_UNDECLARED_OUTPUTS_DIR: artifacts, - VRT_CAPTURE_OUTPUT: job.capture ? path.join(output, 'baselines') : '', - }, - stdio: 'inherit', -}) -if (result.error) console.error(result.error) -// Return a successful build action even when tests fail, so Bazel downloads -// their reports and screenshots. The local test wrapper returns this status. -fs.writeFileSync(path.join(output, 'result.json'), JSON.stringify({ - schemaVersion: 1, - mode: job.capture ? 'capture' : 'compare', - exitCode: result.status ?? 1, -})) -fs.rmSync(temp, {recursive: true, force: true}) +const runtime = bootstrapRuntime(job.runtime) +runRemoteJob(job, runtime.node, {LD_LIBRARY_PATH: runtime.libraryPath}) diff --git a/runtime/runner.ts b/runtime/runner.ts index e093c69..921aa0e 100644 --- a/runtime/runner.ts +++ b/runtime/runner.ts @@ -11,6 +11,7 @@ import {baselineDestination, updateBaselines} from './baselines.js' import {stageRunfiles, testEnvironment} from './isolation.js' import {hostBrowserEnvironment} from './host-browser.js' import {browserRuntime, type BrowserRuntime} from './browser-runtime.js' +import {relocateInputs, runtimeDirectory} from './relocation.js' function required(name: string) { const value = process.env[name] @@ -68,6 +69,7 @@ async function main() { ? browserRuntime(inputs, descriptor.browser) : undefined const node = declaredBrowser?.node || fs.realpathSync(required('JS_BINARY__NODE_BINARY')) + if (visual) relocateInputs(inputs) const testRoot = path.dirname(descriptorPath) const generated = path.join(testRoot, '.rules-browser') fs.mkdirSync(generated) @@ -147,6 +149,13 @@ async function main() { ), ...hostEnv, ...declaredBrowser?.env, + ...(visual ? { + VRT_BASH: path.join(runtimeDirectory, 'bash'), + NODE_OPTIONS: [ + process.env.NODE_OPTIONS || '', + `--require=${JSON.stringify(fileURLToPath(new URL('./vrt-processes.cjs', import.meta.url)))}`, + ].filter(Boolean).join(' '), + } : {}), VRT_INPUTS: inputs, VRT_MODE: required('VRT_MODE'), VRT_TEST_ROOT: visual ? testRoot : inputs, diff --git a/runtime/vrt-processes.cjs b/runtime/vrt-processes.cjs new file mode 100644 index 0000000..05a8f27 --- /dev/null +++ b/runtime/vrt-processes.cjs @@ -0,0 +1,17 @@ +// Loaded only in VRT subprocesses. Playwright requests shell:true for webServer; +// Node otherwise hardcodes /bin/sh even when a declared Bash is on PATH. +const childProcess = require('node:child_process') +const {syncBuiltinESMExports} = require('node:module') +const spawn = childProcess.spawn +childProcess.spawn = function (command, args, options) { + if (!Array.isArray(args)) { + options = options ?? args + args = [] + } + if (options?.shell === true) { + if (!process.env.VRT_BASH) throw new Error('VRT subprocess shell is not configured') + options = {...options, shell: process.env.VRT_BASH} + } + return spawn.call(this, command, args, options) +} +syncBuiltinESMExports() From 67f7b10f95e51653bb93f8a89be54e4a00cfe4b0 Mon Sep 17 00:00:00 2001 From: Long Ho Date: Mon, 14 Sep 2026 01:43:22 +0000 Subject: [PATCH 2/2] fix: keep VRT library setup inside the isolated action --- docs/actiond-migration.md | 5 +++-- docs/actiond.md | 2 ++ experiments/actiond/prepare-public.mjs | 7 +++++++ experiments/actiond/run-public-actiond.sh | 15 +++++++++++++++ internal/remote.bzl | 9 +++++++-- runtime/relocation.ts | 6 ++++++ 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/docs/actiond-migration.md b/docs/actiond-migration.md index 81bff9c..2b9a2ba 100644 --- a/docs/actiond-migration.md +++ b/docs/actiond-migration.md @@ -34,6 +34,7 @@ migrations have landed. checks stay in host E2E targets; inherited environment and origin exceptions do not carry over. -The memory-advice patch is upstream PR #48; the declared-rootfs patch remains -isolated locally for upstreaming. Native macOS VM execution and cross-architecture +The memory-advice patch is covered by upstream actiond PR #33. VRT now relocates +declared executable copies and no longer requires the rootfs patch in actiond +PR #49. Native macOS VM execution and cross-architecture pixel equivalence remain unvalidated. These baselines require a Linux amd64 worker. diff --git a/docs/actiond.md b/docs/actiond.md index f3e71ec..c19a347 100644 --- a/docs/actiond.md +++ b/docs/actiond.md @@ -36,6 +36,8 @@ for these baselines. The validated CI worker uses 6 GiB RAM for at most two concurrent actions; size workers for fixture and staging memory as well as Chromium. The macOS native VM backend is not yet validated by these checks. Keep VRT's explicit remote strategies and local fallback disabled. +The bootstrap also rejects ordinary host roots with system shell/loader paths +before creating temporary runtime launchers. ```sh bazel test --config=vrt //path:visual_test diff --git a/experiments/actiond/prepare-public.mjs b/experiments/actiond/prepare-public.mjs index e2451a5..5ddbd16 100644 --- a/experiments/actiond/prepare-public.mjs +++ b/experiments/actiond/prepare-public.mjs @@ -38,6 +38,13 @@ browser_runtime( library_dirs = ["lib"], fontconfig = "etc/fonts", ) +visual_test( + name = "actiond_local_rejection_test", + browser = ":actiond_browser", + config = ":native_config", + tests = ":native_visual_specs", + baseline_dir = "__actiond_local_rejection__", +) visual_test( name = "actiond_native_test", browser = ":actiond_browser", diff --git a/experiments/actiond/run-public-actiond.sh b/experiments/actiond/run-public-actiond.sh index c429531..96b7c1c 100644 --- a/experiments/actiond/run-public-actiond.sh +++ b/experiments/actiond/run-public-actiond.sh @@ -19,6 +19,21 @@ flags=( --noremote_cache_compression --remote_download_outputs=all ) bazel_cmd=("${ACTIOND_BAZEL:-bazelisk}" --output_base="$work/public-bazel-output") +# This target is never executed remotely, so it cannot reuse a successful +# capture from the action cache and accidentally skip the rejection check. +mkdir -p "$work/results" +if "${bazel_cmd[@]}" build //:actiond_local_rejection_test_capture \ + --remote_executor= --remote_cache= --disk_cache= --spawn_strategy=sandboxed,local \ + > "$work/results/local-rejection.log" 2>&1; then + echo 'VRT unexpectedly executed on the host' >&2 + exit 1 +fi +python3 - "$work/results/local-rejection.log" <<'PY' +from pathlib import Path +import sys +log = Path(sys.argv[1]).read_text() +assert 'VRT requires an isolated action without system runtimes' in log, log +PY "${bazel_cmd[@]}" run //:actiond_native_test.update "${flags[@]}" "${bazel_cmd[@]}" run //:actiond_gallery_test.update "${flags[@]}" test -s __actiond_native__/saved.png diff --git a/internal/remote.bzl b/internal/remote.bzl index 831bfaf..f87a45b 100644 --- a/internal/remote.bzl +++ b/internal/remote.bzl @@ -48,7 +48,13 @@ def _remote_impl(ctx): descriptor = browser.descriptor ctx.actions.run( executable = root.path + "/" + descriptor["loader"], - arguments = [root.path + "/" + descriptor["node"], ctx.file._bootstrap.path, job.path], + arguments = [ + "--library-path", + ":".join([root.path + "/" + p for p in descriptor["libraryDirs"]]), + root.path + "/" + descriptor["node"], + ctx.file._bootstrap.path, + job.path, + ], inputs = depset(files + [root, job, ctx.file._bootstrap]), outputs = [output], env = { @@ -56,7 +62,6 @@ def _remote_impl(ctx): "TMPDIR": "/tmp", "LANG": "C.UTF-8", "TZ": "UTC", - "LD_LIBRARY_PATH": ":".join([root.path + "/" + p for p in descriptor["libraryDirs"]]), }, execution_requirements = {"no-local": "1"}, mnemonic = "VrtCapture" if ctx.attr.capture else "VrtCompare", diff --git a/runtime/relocation.ts b/runtime/relocation.ts index 70552d2..c092e8a 100644 --- a/runtime/relocation.ts +++ b/runtime/relocation.ts @@ -65,6 +65,12 @@ export function relocateInputs(directory: string): void { export function bootstrapRuntime(runtime: { path: string; loader: string; node: string; bash: string; libraryDirs: string[] }) { + // Bazel's no-local requirement does not exclude every local spawn strategy. + // Reject ordinary host roots before creating the action-private launchers. + for (const file of ['/bin/sh', '/usr/bin/env', '/lib64/ld-linux-x86-64.so.2']) { + if (fs.existsSync(file)) + throw new Error('VRT requires an isolated action without system runtimes; select the remote VRT configuration') + } const root = fs.realpathSync(runtime.path) const resolve = (relative: string) => { if (!relative || path.isAbsolute(relative) || relative.split('/').some(p => !p || p === '.' || p === '..'))