From c01ef5e18b0c8ee5cb459f9f5d0080a7968483b7 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 07:37:56 -0700 Subject: [PATCH 1/2] fix(microvm): filter workflow commands from guest output Neutralize untrusted Actions command syntax at the runner-facing boundary. Retain bounded raw guest output for diagnostics and audit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/cloud-hypervisor-foundation.md | 28 ++++ src/cloud-hypervisor-runtime-backend.test.ts | 50 +++++++ src/cloud-hypervisor-runtime-backend.ts | 5 + src/cloud-hypervisor/diagnostics.ts | 37 +++++ src/cloud-hypervisor/manager.test.ts | 24 ++++ src/cloud-hypervisor/manager.ts | 22 ++- src/microvm/vsock-client.test.ts | 104 ++++++++++++++ src/microvm/vsock-client.ts | 84 +++++++++-- src/microvm/workflow-command-filter.test.ts | 115 +++++++++++++++ src/microvm/workflow-command-filter.ts | 140 +++++++++++++++++++ 10 files changed, 594 insertions(+), 15 deletions(-) create mode 100644 src/microvm/workflow-command-filter.test.ts create mode 100644 src/microvm/workflow-command-filter.ts diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index c797003ec..79c7151cf 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -144,6 +144,34 @@ direct internet, arbitrary TCP, direct DNS, and instance metadata access. Guest proxy environment variables improve client compatibility, but the namespace policy is the enforcement boundary. +### Untrusted guest output + +Guest stdout and stderr cross a host-side presentation boundary before AWF writes +them to the runner log. A streaming byte filter neutralizes lines that begin, +after optional runner-recognized leading whitespace, with GitHub Actions workflow-command +syntax such as `::set-output::`, `::add-mask::`, or `::stop-commands::`. The +filter operates across VSOCK frame boundaries, preserves non-command and +non-UTF-8 bytes, and buffers at most two candidate bytes rather than a complete +line. Output writes continue to honor stream backpressure. + +AWF intentionally has no workflow-command allowlist for guest output. Unlike a +trusted host helper, the guest cannot prove that an informational annotation +such as `::error::` came from a trusted producer, so allowing any command name +would preserve an unnecessary runner-control channel. + +Filtering applies only to the live runner-facing stdout and stderr streams. +Internal readiness probes retain their original bytes and semantics. Before +filtering, AWF also captures the exact raw guest streams in bounded 1 MiB tails. +Diagnostic collection writes these private files with mode `0600`: + +- `guest-stdout.raw.log` +- `guest-stderr.raw.log` + +They are stored alongside the other Cloud Hypervisor diagnostics under the +configured audit directory, or under the work-directory diagnostics path when +no audit directory is configured. This preserves forensic evidence without +allowing raw guest bytes to reach the GitHub Actions command parser. + ## Guest and workspace The guest boots a pinned PCI-capable Linux kernel and deterministic BusyBox diff --git a/src/cloud-hypervisor-runtime-backend.test.ts b/src/cloud-hypervisor-runtime-backend.test.ts index 87ad328fe..fa584eb83 100644 --- a/src/cloud-hypervisor-runtime-backend.test.ts +++ b/src/cloud-hypervisor-runtime-backend.test.ts @@ -139,6 +139,7 @@ function harness(overrides: Partial = writeStdin: jest.fn().mockResolvedValue(undefined), endStdin: jest.fn().mockResolvedValue(undefined), collectDiagnostics: jest.fn().mockResolvedValue(undefined), + collectGuestOutputAudit: jest.fn().mockResolvedValue(undefined), stop: jest.fn(async (options?: { beforeCleanup?: () => Promise }) => { order.push('vm-stop'); await options?.beforeCleanup?.(); @@ -353,11 +354,60 @@ describe('Cloud Hypervisor runtime backend', () => { uid: 1000, gid: 1000, timeoutMs: 60_000, + filterWorkflowCommands: true, })); + expect(manager.execute).toHaveBeenNthCalledWith( + 1, + expect.not.objectContaining({ filterWorkflowCommands: true }), + ); + expect(manager.execute).toHaveBeenNthCalledWith( + 2, + expect.not.objectContaining({ filterWorkflowCommands: true }), + ); expect(manager.writeStdin).toHaveBeenCalledWith( Buffer.from('input'), expect.stringMatching(/^agent-/), ); + expect(manager.collectGuestOutputAudit).not.toHaveBeenCalled(); + }); + + it('persists bounded raw guest output when an audit directory is configured', async () => { + const { manager, deps, stdin } = harness(); + const backend = createBackend(config({ auditDir: '/tmp/audit' }), deps); + + await backend.start('/tmp/awf', ['github.com']); + const execution = backend.exec('/tmp/awf', ['github.com']); + stdin.end(); + await expect(execution).resolves.toEqual({ exitCode: 23 }); + + expect(manager.collectGuestOutputAudit).toHaveBeenCalledWith( + '/tmp/audit/cloud-hypervisor', + ); + }); + + it('persists raw guest output after a failed agent execution', async () => { + const { manager, deps, stdin } = harness(); + manager.execute.mockReset().mockImplementationOnce(async (request) => ({ + requestId: request.requestId, + exitCode: 0, + signal: null, + timedOut: false, + })).mockImplementationOnce(async (request) => ({ + requestId: request.requestId, + exitCode: 0, + signal: null, + timedOut: false, + })).mockRejectedValueOnce(new Error('guest transport failed')); + const backend = createBackend(config({ auditDir: '/tmp/audit' }), deps); + + await backend.start('/tmp/awf', ['github.com']); + const execution = backend.exec('/tmp/awf', ['github.com']); + stdin.end(); + await expect(execution).rejects.toThrow('guest transport failed'); + + expect(manager.collectGuestOutputAudit).toHaveBeenCalledWith( + '/tmp/audit/cloud-hypervisor', + ); }); it('discovers and probes trusted topology peers before agent execution', async () => { diff --git a/src/cloud-hypervisor-runtime-backend.ts b/src/cloud-hypervisor-runtime-backend.ts index 6c3678c0f..125c91c62 100644 --- a/src/cloud-hypervisor-runtime-backend.ts +++ b/src/cloud-hypervisor-runtime-backend.ts @@ -93,6 +93,7 @@ interface CloudHypervisorManagerAdapter { endStdin(requestId?: string): Promise; stop(options?: { preserve?: boolean; beforeCleanup?: () => Promise }): Promise; collectDiagnostics(directory: string): Promise; + collectGuestOutputAudit(directory: string): Promise; } /** @internal Exposed only for unit tests — not part of the public API. */ @@ -400,6 +401,7 @@ class CloudHypervisorRuntimeBackend implements ExternalAgentRuntimeBackend { ...(timeoutMs === undefined ? {} : { timeoutMs }), stdout: this.dependencies.stdout, stderr: this.dependencies.stderr, + filterWorkflowCommands: true, }); this.activeExecution = { requestId, promise: execution }; @@ -438,6 +440,9 @@ class CloudHypervisorRuntimeBackend implements ExternalAgentRuntimeBackend { this.dependencies.stdin.off('end', onEnd); await forwarding; this.activeExecution = undefined; + if (this.config.auditDir) { + await manager.collectGuestOutputAudit(`${this.config.auditDir}/cloud-hypervisor`); + } } }; diff --git a/src/cloud-hypervisor/diagnostics.ts b/src/cloud-hypervisor/diagnostics.ts index d6df5fa93..d09956e6e 100644 --- a/src/cloud-hypervisor/diagnostics.ts +++ b/src/cloud-hypervisor/diagnostics.ts @@ -1,5 +1,6 @@ import { constants, promises as fs } from 'fs'; import * as path from 'path'; +import { Writable } from 'stream'; import type { ExecaChildProcess } from 'execa'; import type { MicrovmNetworkLifecycle, MicrovmNetworkPlan } from '../microvm/network'; import { @@ -55,6 +56,15 @@ export class BoundedOutputCapture { contents(): Buffer { return this.buffer; } + + writable(): Writable { + return new Writable({ + write: (chunk: Buffer, _encoding, callback) => { + this.append(chunk); + callback(); + }, + }); + } } /** @@ -151,6 +161,8 @@ export interface CloudHypervisorDiagnosticsContext { config: CloudHypervisorOptions; stdoutCapture: BoundedOutputCapture; stderrCapture: BoundedOutputCapture; + guestStdoutCapture: BoundedOutputCapture; + guestStderrCapture: BoundedOutputCapture; network: MicrovmNetworkLifecycle | undefined; networkPlan: MicrovmNetworkPlan | undefined; client: CloudHypervisorApiClient | undefined; @@ -162,6 +174,25 @@ export interface CloudHypervisorDiagnosticsContext { fsDevices: readonly VirtiofsdDevice[]; } +export async function writeGuestOutputAudit( + directory: string, + dependencies: Pick, + stdoutCapture: BoundedOutputCapture, + stderrCapture: BoundedOutputCapture, +): Promise { + await dependencies.mkdir(directory, { recursive: true, mode: 0o700 }); + await dependencies.writeFile( + path.join(directory, 'guest-stdout.raw.log'), + stdoutCapture.contents(), + { mode: 0o600 }, + ); + await dependencies.writeFile( + path.join(directory, 'guest-stderr.raw.log'), + stderrCapture.contents(), + { mode: 0o600 }, + ); +} + export async function collectCloudHypervisorDiagnostics( directory: string, context: CloudHypervisorDiagnosticsContext, @@ -198,6 +229,12 @@ export async function collectCloudHypervisorDiagnostics( }; await writeBounded('launcher-stdout.log', context.stdoutCapture.contents()); await writeBounded('launcher-stderr.log', context.stderrCapture.contents()); + await writeGuestOutputAudit( + directory, + dependencies, + context.guestStdoutCapture, + context.guestStderrCapture, + ); await copyBoundedDiagnostic( dependencies, paths.logPath, diff --git a/src/cloud-hypervisor/manager.test.ts b/src/cloud-hypervisor/manager.test.ts index 7ce0e8cea..589c29705 100644 --- a/src/cloud-hypervisor/manager.test.ts +++ b/src/cloud-hypervisor/manager.test.ts @@ -489,6 +489,30 @@ describe('CloudHypervisorManager', () => { uid: 1000, gid: 1000, })).resolves.toEqual(expect.objectContaining({ exitCode: 0 })); + const forwardedRequest = (guestClient.execute as jest.Mock).mock.calls[0][0]; + const rawGuestStdout = Buffer.concat([ + Buffer.from('discarded prefix'), + Buffer.alloc(1024 * 1024, 0x7a), + ]); + forwardedRequest.rawStdout.write(rawGuestStdout); + forwardedRequest.rawStderr.write(Buffer.from([0xff, 0x00, 0xfe])); + await manager.collectDiagnostics('/tmp/diagnostics'); + expect(deps.writeFile).toHaveBeenCalledWith( + '/tmp/diagnostics/guest-stdout.raw.log', + Buffer.alloc(1024 * 1024, 0x7a), + { mode: 0o600 }, + ); + expect(deps.writeFile).toHaveBeenCalledWith( + '/tmp/diagnostics/guest-stderr.raw.log', + Buffer.from([0xff, 0x00, 0xfe]), + { mode: 0o600 }, + ); + await manager.collectGuestOutputAudit('/tmp/audit/cloud-hypervisor'); + expect(deps.writeFile).toHaveBeenCalledWith( + '/tmp/audit/cloud-hypervisor/guest-stdout.raw.log', + Buffer.alloc(1024 * 1024, 0x7a), + { mode: 0o600 }, + ); await manager.stop(); expect(guestClient.shutdown).toHaveBeenCalledTimes(1); diff --git a/src/cloud-hypervisor/manager.ts b/src/cloud-hypervisor/manager.ts index b6d4b0f9b..69be49a80 100644 --- a/src/cloud-hypervisor/manager.ts +++ b/src/cloud-hypervisor/manager.ts @@ -25,6 +25,7 @@ import { BoundedOutputCapture, collectCloudHypervisorDiagnostics, readBoundedTail, + writeGuestOutputAudit, } from './diagnostics'; import { CloudHypervisorGuestChannel, @@ -169,6 +170,10 @@ export class CloudHypervisorManager { private lastVmCounters: CloudHypervisorVmCounters | undefined; private readonly stdoutCapture = new BoundedOutputCapture(CLOUD_HYPERVISOR_CAPTURE_LIMIT_BYTES); private readonly stderrCapture = new BoundedOutputCapture(CLOUD_HYPERVISOR_CAPTURE_LIMIT_BYTES); + private readonly guestStdoutCapture = new BoundedOutputCapture(CLOUD_HYPERVISOR_CAPTURE_LIMIT_BYTES); + private readonly guestStderrCapture = new BoundedOutputCapture(CLOUD_HYPERVISOR_CAPTURE_LIMIT_BYTES); + private readonly guestStdoutAudit = this.guestStdoutCapture.writable(); + private readonly guestStderrAudit = this.guestStderrCapture.writable(); get guestIp(): string | undefined { return this.networkPlan?.guestIp; @@ -244,7 +249,11 @@ export class CloudHypervisorManager { if (!this.guest) { throw new Error('Cloud Hypervisor guest supervisor is not ready'); } - return this.guest.execute(request); + return this.guest.execute({ + ...request, + rawStdout: this.guestStdoutAudit, + rawStderr: this.guestStderrAudit, + }); } cancel(reason = 'host cancellation', requestId?: string): Promise { @@ -315,6 +324,8 @@ export class CloudHypervisorManager { config: this.config, stdoutCapture: this.stdoutCapture, stderrCapture: this.stderrCapture, + guestStdoutCapture: this.guestStdoutCapture, + guestStderrCapture: this.guestStderrCapture, network: this.network, networkPlan: this.networkPlan, client: this.client, @@ -324,4 +335,13 @@ export class CloudHypervisorManager { fsDevices: this.fsDevices, }); } + + async collectGuestOutputAudit(directory: string): Promise { + await writeGuestOutputAudit( + directory, + this.dependencies, + this.guestStdoutCapture, + this.guestStderrCapture, + ); + } } diff --git a/src/microvm/vsock-client.test.ts b/src/microvm/vsock-client.test.ts index d14880d2e..9c06190a2 100644 --- a/src/microvm/vsock-client.test.ts +++ b/src/microvm/vsock-client.test.ts @@ -614,6 +614,110 @@ describe('MicrovmVsockClient', () => { await server.close(); }); + it('filters only presented output while preserving exact raw audit bytes', async () => { + const rawChunks = [ + Buffer.from('plain\r\n :'), + Buffer.from(':set-output name=result::owned\n'), + Buffer.from([0xff, 0xfe, 0x0a]), + ]; + const server = await createServer((frame, socket) => { + if (frame.type !== 'execute') return; + for (const chunk of rawChunks) { + socket.write(encodeGuestFrame({ + version: 1, + type: 'stdout', + requestId: frame.requestId, + data: chunk.toString('base64'), + })); + } + socket.write(encodeGuestFrame({ + version: 1, + type: 'result', + requestId: frame.requestId, + exitCode: 0, + signal: null, + timedOut: false, + })); + }); + const presented: Buffer[] = []; + const audited: Buffer[] = []; + const output = new Writable({ + highWaterMark: 1, + write(chunk: Buffer, _encoding, callback) { + presented.push(Buffer.from(chunk)); + setImmediate(callback); + }, + }); + const rawOutput = new Writable({ + highWaterMark: 1, + write(chunk: Buffer, _encoding, callback) { + audited.push(Buffer.from(chunk)); + setImmediate(callback); + }, + }); + const client = new MicrovmVsockClient({ + socketPath: server.socketPath, + guestPort: 52, + }); + + await client.connect(); + await expect(client.execute({ + requestId: 'filtered', + argv: ['true'], + env: {}, + cwd: '/workspace', + uid: 1000, + gid: 1000, + stdout: output, + rawStdout: rawOutput, + filterWorkflowCommands: true, + })).resolves.toEqual(expect.objectContaining({ exitCode: 0 })); + + expect(Buffer.concat(audited)).toEqual(Buffer.concat(rawChunks)); + expect(Buffer.concat(presented)).toEqual(Buffer.concat([ + Buffer.from('plain\r\n [awf blocked workflow command] : :set-output name=result::owned\n'), + Buffer.from([0xff, 0xfe, 0x0a]), + ])); + client.destroy(); + await server.close(); + }); + + it('flushes a trailing command candidate when cancellation grace expires', async () => { + const server = await createServer((frame, socket) => { + if (frame.type !== 'execute') return; + socket.write(encodeGuestFrame({ + version: 1, + type: 'stdout', + requestId: frame.requestId, + data: Buffer.from('trailing::').toString('base64'), + })); + }); + const output = new PassThrough(); + const chunks: Buffer[] = []; + output.on('data', (chunk) => chunks.push(chunk)); + const client = new MicrovmVsockClient({ + socketPath: server.socketPath, + guestPort: 52, + cancellationGraceMs: 5, + }); + + await client.connect(); + await expect(client.execute({ + requestId: 'timeout-filter', + argv: ['sleep', '10'], + env: {}, + cwd: '/workspace', + uid: 1000, + gid: 1000, + timeoutMs: 5, + stdout: output, + filterWorkflowCommands: true, + })).resolves.toEqual(expect.objectContaining({ exitCode: 124 })); + + expect(Buffer.concat(chunks).toString()).toBe('trailing::'); + await server.close(); + }); + it('rejects writes after a connected transport is destroyed', async () => { const server = await createServer(() => undefined); const client = new MicrovmVsockClient({ diff --git a/src/microvm/vsock-client.ts b/src/microvm/vsock-client.ts index 990431ef3..c078f2526 100644 --- a/src/microvm/vsock-client.ts +++ b/src/microvm/vsock-client.ts @@ -14,6 +14,7 @@ import { type GuestReadyFrame, type GuestResultFrame, } from './guest-protocol'; +import { WorkflowCommandFilter } from './workflow-command-filter'; const GUEST_VSOCK_HANDSHAKE_LIMIT = 128; @@ -37,6 +38,9 @@ export interface GuestExecutionRequest { readonly requestId?: string; readonly stdout?: Writable; readonly stderr?: Writable; + readonly rawStdout?: Writable; + readonly rawStderr?: Writable; + readonly filterWorkflowCommands?: boolean; } export interface GuestExecutionResult { @@ -50,6 +54,10 @@ interface PendingExecution { readonly requestId: string; readonly stdout?: Writable; readonly stderr?: Writable; + readonly rawStdout?: Writable; + readonly rawStderr?: Writable; + readonly stdoutFilter?: WorkflowCommandFilter; + readonly stderrFilter?: WorkflowCommandFilter; readonly resolve: (result: GuestExecutionResult) => void; readonly reject: (error: Error) => void; hostTimedOut: boolean; @@ -164,6 +172,14 @@ export class MicrovmVsockClient { requestId, stdout: request.stdout, stderr: request.stderr, + rawStdout: request.rawStdout, + rawStderr: request.rawStderr, + stdoutFilter: request.filterWorkflowCommands && request.stdout + ? new WorkflowCommandFilter() + : undefined, + stderrFilter: request.filterWorkflowCommands && request.stderr + ? new WorkflowCommandFilter() + : undefined, resolve, reject, hostTimedOut: false, @@ -180,15 +196,18 @@ export class MicrovmVsockClient { }).catch((error) => this.fail(toError(error))); pending.cancellation = setTimeout(() => { if (this.pending !== pending) return; - this.completePending({ - version: GUEST_PROTOCOL_VERSION, - type: 'result', - requestId, - exitCode: 124, - signal: null, - timedOut: true, - }); - this.socket?.destroy(); + void this.flushPendingOutput(pending).then(() => { + if (this.pending !== pending) return; + this.completePending({ + version: GUEST_PROTOCOL_VERSION, + type: 'result', + requestId, + exitCode: 124, + signal: null, + timedOut: true, + }); + this.socket?.destroy(); + }).catch((error) => this.fail(toError(error))); }, this.cancellationGraceMs); }, request.timeoutMs); } @@ -313,6 +332,7 @@ export class MicrovmVsockClient { if (frame.type === 'error') { const error = new GuestExecutionError(frame); if (this.pending?.requestId === frame.requestId) { + await this.flushPendingOutput(this.pending); this.rejectPending(error); } else { this.fail(error); @@ -321,12 +341,20 @@ export class MicrovmVsockClient { } if (frame.type === 'stdout' || frame.type === 'stderr') { const pending = this.requirePending(frame.requestId); + const data = Buffer.from(frame.data, 'base64'); const destination = frame.type === 'stdout' ? pending.stdout : pending.stderr; - if (destination) await writeWithBackpressure(destination, Buffer.from(frame.data, 'base64')); + const rawDestination = frame.type === 'stdout' ? pending.rawStdout : pending.rawStderr; + const filter = frame.type === 'stdout' ? pending.stdoutFilter : pending.stderrFilter; + if (rawDestination) await writeWithBackpressure(rawDestination, data); + if (destination) { + const presented = filter?.push(data) ?? data; + if (presented.length > 0) await writeWithBackpressure(destination, presented); + } return; } if (frame.type === 'result') { - this.requirePending(frame.requestId); + const pending = this.requirePending(frame.requestId); + await this.flushPendingOutput(pending); this.completePending(frame); return; } @@ -365,6 +393,7 @@ export class MicrovmVsockClient { }); return; } + pending.resolve({ requestId: frame.requestId, exitCode: frame.exitCode ?? 128 + signalNumber(frame.signal), @@ -373,6 +402,18 @@ export class MicrovmVsockClient { }); } + private async flushPendingOutput(pending: PendingExecution): Promise { + const streams = [ + [pending.stdout, pending.stdoutFilter], + [pending.stderr, pending.stderrFilter], + ] as const; + for (const [destination, filter] of streams) { + if (!destination || !filter) continue; + const trailing = filter.finish(); + if (trailing.length > 0) await writeWithBackpressure(destination, trailing); + } + } + private rejectPending(error: Error): void { if (!this.pending) return; clearTimeout(this.pending.timeout); @@ -434,10 +475,25 @@ export class MicrovmVsockClient { } async function writeWithBackpressure(destination: Writable, data: Buffer): Promise { - if (destination.write(data)) return; await new Promise((resolve, reject) => { - destination.once('drain', resolve); - destination.once('error', reject); + const cleanup = (): void => { + destination.off('drain', onDrain); + destination.off('error', onError); + }; + const onDrain = (): void => { + cleanup(); + resolve(); + }; + const onError = (error: Error): void => { + cleanup(); + reject(error); + }; + destination.once('drain', onDrain); + destination.once('error', onError); + if (destination.write(data)) { + cleanup(); + resolve(); + } }); } diff --git a/src/microvm/workflow-command-filter.test.ts b/src/microvm/workflow-command-filter.test.ts new file mode 100644 index 000000000..97cb52b3e --- /dev/null +++ b/src/microvm/workflow-command-filter.test.ts @@ -0,0 +1,115 @@ +import { WorkflowCommandFilter } from './workflow-command-filter'; + +function filter(chunks: readonly Buffer[]): Buffer { + const subject = new WorkflowCommandFilter(); + return Buffer.concat([...chunks.map((chunk) => subject.push(chunk)), subject.finish()]); +} + +describe('WorkflowCommandFilter', () => { + it('preserves ordinary and non-UTF-8 output byte-for-byte', () => { + const input = Buffer.from([ + ...Buffer.from('plain :: text\n:not-a-command\n::1-not-a-command\r\n'), + 0xff, + 0xfe, + 0x00, + 0x3a, + ]); + + expect(filter([input])).toEqual(input); + }); + + it('neutralizes workflow command syntax after leading whitespace on LF and CRLF lines', () => { + const input = Buffer.from( + '::set-output name=result::owned\n' + + '\t ::stop-commands::token\r\n' + + ' ::error file=x::untrusted annotation\r' + + 'safe::set-env::mid-line\n', + ); + + expect(filter([input]).toString()).toBe( + '[awf blocked workflow command] : :set-output name=result::owned\n' + + '\t [awf blocked workflow command] : :stop-commands::token\r\n' + + ' [awf blocked workflow command] : :error file=x::untrusted annotation\r' + + 'safe::set-env::mid-line\n', + ); + }); + + it('recognizes command starts split at every relevant chunk boundary', () => { + const chunks = [ + Buffer.from(' \t:'), + Buffer.from(':'), + Buffer.from('s'), + Buffer.from('et-env::owned\r'), + Buffer.from('\n:'), + Buffer.from(':add-mask::secret'), + ]; + + expect(filter(chunks).toString()).toBe( + ' \t[awf blocked workflow command] : :set-env::owned\r\n' + + '[awf blocked workflow command] : :add-mask::secret', + ); + }); + + it('matches the runner whitespace trim across split UTF-8 sequences', () => { + const whitespace = [ + '\u0085', + '\u00a0', + '\u1680', + '\u2000', + '\u200a', + '\u2028', + '\u2029', + '\u202f', + '\u205f', + '\u3000', + ].join(''); + const input = Buffer.from(`${whitespace}::stop-commands::token\n`); + const chunks = Array.from(input, (byte) => Buffer.from([byte])); + + expect(filter(chunks).toString()).toBe( + `${whitespace}[awf blocked workflow command] : :stop-commands::token\n`, + ); + }); + + it('preserves invalid UTF-8 whitespace candidates byte-for-byte', () => { + const input = Buffer.from([0xe2, 0x0a, 0x3a, 0x3a, 0x31, 0xff]); + expect(filter(Array.from(input, (byte) => Buffer.from([byte])))).toEqual(input); + }); + + it('flushes incomplete candidates without changing them', () => { + expect(filter([Buffer.from('one\n:')]).toString()).toBe('one\n:'); + expect(filter([Buffer.from('two\n::')]).toString()).toBe('two\n::'); + }); + + it('does not rescan later text after a false command candidate', () => { + for (const input of [ + ': ::warning::ordinary text\n', + ':: ::warning::ordinary text\n', + '::1 ::warning::ordinary text\n', + ]) { + expect(filter([Buffer.from(input)]).toString()).toBe(input); + } + }); + + it('streams very long lines without retaining the line', () => { + const payload = Buffer.alloc(2 * 1024 * 1024, 0x61); + const chunks = [ + Buffer.from('::set-output::'), + ...Array.from( + { length: Math.ceil(payload.length / 8191) }, + (_, index) => payload.subarray(index * 8191, (index + 1) * 8191), + ), + Buffer.from('\n'), + ]; + const output = filter(chunks); + + expect(output.subarray(0, 48).toString()).toContain( + '[awf blocked workflow command] : :set-output::', + ); + expect(output.length).toBe( + payload.length + + Buffer.byteLength('[awf blocked workflow command] : :set-output::\n'), + ); + expect(output.subarray(-2)).toEqual(Buffer.from('a\n')); + }); +}); diff --git a/src/microvm/workflow-command-filter.ts b/src/microvm/workflow-command-filter.ts new file mode 100644 index 000000000..7180b04ab --- /dev/null +++ b/src/microvm/workflow-command-filter.ts @@ -0,0 +1,140 @@ +const BLOCKED_COMMAND_PREFIX = Buffer.from('[awf blocked workflow command] ', 'ascii'); +const NEUTRALIZED_COMMAND_START = Buffer.from(': :', 'ascii'); + +function isAsciiWhitespace(byte: number): boolean { + return byte === 0x09 || byte === 0x0b || byte === 0x0c || byte === 0x20; +} + +function isUnicodeWhitespaceLead(byte: number): boolean { + return byte === 0xc2 || byte === 0xe1 || byte === 0xe2 || byte === 0xe3; +} + +function unicodeWhitespaceStatus( + bytes: readonly number[], +): 'pending' | 'whitespace' | 'invalid' { + const [first, second, third] = bytes; + if (first === 0xc2) { + if (bytes.length === 1) return 'pending'; + return second === 0x85 || second === 0xa0 ? 'whitespace' : 'invalid'; + } + if (first === 0xe1) { + if (bytes.length === 1) return 'pending'; + if (second !== 0x9a) return 'invalid'; + if (bytes.length === 2) return 'pending'; + return third === 0x80 ? 'whitespace' : 'invalid'; + } + if (first === 0xe2) { + if (bytes.length === 1) return 'pending'; + if (second !== 0x80 && second !== 0x81) return 'invalid'; + if (bytes.length === 2) return 'pending'; + if (second === 0x80) { + return ( + (third !== undefined && third >= 0x80 && third <= 0x8a) || + third === 0xa8 || + third === 0xa9 || + third === 0xaf + ) ? 'whitespace' : 'invalid'; + } + return third === 0x9f ? 'whitespace' : 'invalid'; + } + if (first === 0xe3) { + if (bytes.length === 1) return 'pending'; + if (second !== 0x80) return 'invalid'; + if (bytes.length === 2) return 'pending'; + return third === 0x80 ? 'whitespace' : 'invalid'; + } + return 'invalid'; +} + +function isAsciiLetter(byte: number): boolean { + return (byte >= 0x41 && byte <= 0x5a) || (byte >= 0x61 && byte <= 0x7a); +} + +/** + * Neutralizes GitHub Actions workflow commands in an untrusted byte stream. + * It buffers at most two candidate bytes, so arbitrary chunks and long lines + * do not require decoding or line-sized buffering. + */ +export class WorkflowCommandFilter { + private lineCanStartCommand = true; + private candidateColons = 0; + private unicodeWhitespaceCandidate: number[] = []; + + push(data: Buffer): Buffer { + const output: number[] = []; + + for (const byte of data) { + this.processByte(byte, output); + } + + return Buffer.from(output); + } + + finish(): Buffer { + const trailing = Buffer.from([ + ...this.unicodeWhitespaceCandidate, + ...Array(this.candidateColons).fill(0x3a), + ]); + this.unicodeWhitespaceCandidate = []; + this.candidateColons = 0; + return trailing; + } + + private processByte(byte: number, output: number[]): void { + if (this.unicodeWhitespaceCandidate.length > 0) { + this.unicodeWhitespaceCandidate.push(byte); + const status = unicodeWhitespaceStatus(this.unicodeWhitespaceCandidate); + if (status === 'pending') return; + const candidate = this.unicodeWhitespaceCandidate; + this.unicodeWhitespaceCandidate = []; + if (status === 'whitespace') { + output.push(...candidate); + return; + } + this.lineCanStartCommand = false; + output.push(candidate[0]); + for (const remaining of candidate.slice(1)) this.processByte(remaining, output); + return; + } + + if (this.candidateColons === 1) { + if (byte === 0x3a) { + this.candidateColons = 2; + return; + } + output.push(0x3a); + this.candidateColons = 0; + this.lineCanStartCommand = false; + } else if (this.candidateColons === 2) { + this.candidateColons = 0; + if (isAsciiLetter(byte)) { + output.push(...BLOCKED_COMMAND_PREFIX, ...NEUTRALIZED_COMMAND_START, byte); + this.lineCanStartCommand = false; + return; + } + output.push(0x3a, 0x3a); + this.lineCanStartCommand = false; + } + + if (byte === 0x0a || byte === 0x0d) { + output.push(byte); + this.lineCanStartCommand = true; + return; + } + if (this.lineCanStartCommand && isAsciiWhitespace(byte)) { + output.push(byte); + return; + } + if (this.lineCanStartCommand && isUnicodeWhitespaceLead(byte)) { + this.unicodeWhitespaceCandidate.push(byte); + return; + } + if (this.lineCanStartCommand && byte === 0x3a) { + this.candidateColons = 1; + return; + } + + output.push(byte); + this.lineCanStartCommand = false; + } +} From 7653f5f43869126b15c13d8f4d88a6a296148a6d Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 07:53:23 -0700 Subject: [PATCH 2/2] fix(microvm): block legacy workflow commands Neutralize legacy runner commands and flush filter state on transport failures. Add regression coverage for split commands, disconnects, and stream errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/cloud-hypervisor-foundation.md | 6 +- src/microvm/vsock-client.test.ts | 68 +++++++++++++++++++++ src/microvm/vsock-client.ts | 45 +++++++++----- src/microvm/workflow-command-filter.test.ts | 23 +++++++ src/microvm/workflow-command-filter.ts | 44 ++++++++++++- 5 files changed, 168 insertions(+), 18 deletions(-) diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index 79c7151cf..ca5893e8e 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -151,8 +151,10 @@ them to the runner log. A streaming byte filter neutralizes lines that begin, after optional runner-recognized leading whitespace, with GitHub Actions workflow-command syntax such as `::set-output::`, `::add-mask::`, or `::stop-commands::`. The filter operates across VSOCK frame boundaries, preserves non-command and -non-UTF-8 bytes, and buffers at most two candidate bytes rather than a complete -line. Output writes continue to honor stream backpressure. +non-UTF-8 bytes, and retains only constant-size command candidates rather than +complete lines. It also neutralizes the runner's legacy `##[...]` command form +wherever it appears in a line. Output writes continue to honor stream +backpressure. AWF intentionally has no workflow-command allowlist for guest output. Unlike a trusted host helper, the guest cannot prove that an informational annotation diff --git a/src/microvm/vsock-client.test.ts b/src/microvm/vsock-client.test.ts index 9c06190a2..a0b59dc22 100644 --- a/src/microvm/vsock-client.test.ts +++ b/src/microvm/vsock-client.test.ts @@ -718,6 +718,74 @@ describe('MicrovmVsockClient', () => { await server.close(); }); + it('flushes pending filter bytes before rejecting an abrupt disconnect', async () => { + const server = await createServer((frame, socket) => { + if (frame.type !== 'execute') return; + socket.write(encodeGuestFrame({ + version: 1, + type: 'stdout', + requestId: frame.requestId, + data: Buffer.from('trailing::').toString('base64'), + }), () => socket.destroy()); + }); + const output = new PassThrough(); + const chunks: Buffer[] = []; + output.on('data', (chunk) => chunks.push(chunk)); + const client = new MicrovmVsockClient({ + socketPath: server.socketPath, + guestPort: 52, + }); + + await client.connect(); + await expect(client.execute({ + requestId: 'disconnect-filter', + argv: ['true'], + env: {}, + cwd: '/workspace', + uid: 1000, + gid: 1000, + stdout: output, + filterWorkflowCommands: true, + })).rejects.toThrow(/disconnected/); + + expect(Buffer.concat(chunks).toString()).toBe('trailing::'); + await server.close(); + }); + + it('rejects when a backpressured output stream errors', async () => { + const server = await createServer((frame, socket) => { + if (frame.type !== 'execute') return; + socket.write(encodeGuestFrame({ + version: 1, + type: 'stdout', + requestId: frame.requestId, + data: Buffer.from('output').toString('base64'), + })); + }); + const output = new Writable({ + highWaterMark: 1, + write(_chunk, _encoding, callback) { + callback(new Error('output failed')); + }, + }); + const client = new MicrovmVsockClient({ + socketPath: server.socketPath, + guestPort: 52, + }); + + await client.connect(); + await expect(client.execute({ + requestId: 'output-error', + argv: ['true'], + env: {}, + cwd: '/workspace', + uid: 1000, + gid: 1000, + stdout: output, + })).rejects.toThrow('output failed'); + await server.close(); + }); + it('rejects writes after a connected transport is destroyed', async () => { const server = await createServer(() => undefined); const client = new MicrovmVsockClient({ diff --git a/src/microvm/vsock-client.ts b/src/microvm/vsock-client.ts index c078f2526..4859303b2 100644 --- a/src/microvm/vsock-client.ts +++ b/src/microvm/vsock-client.ts @@ -196,18 +196,7 @@ export class MicrovmVsockClient { }).catch((error) => this.fail(toError(error))); pending.cancellation = setTimeout(() => { if (this.pending !== pending) return; - void this.flushPendingOutput(pending).then(() => { - if (this.pending !== pending) return; - this.completePending({ - version: GUEST_PROTOCOL_VERSION, - type: 'result', - requestId, - exitCode: 124, - signal: null, - timedOut: true, - }); - this.socket?.destroy(); - }).catch((error) => this.fail(toError(error))); + void this.completeTimedOutPending(pending); }, this.cancellationGraceMs); }, request.timeoutMs); } @@ -332,7 +321,6 @@ export class MicrovmVsockClient { if (frame.type === 'error') { const error = new GuestExecutionError(frame); if (this.pending?.requestId === frame.requestId) { - await this.flushPendingOutput(this.pending); this.rejectPending(error); } else { this.fail(error); @@ -414,12 +402,39 @@ export class MicrovmVsockClient { } } + private async completeTimedOutPending(pending: PendingExecution): Promise { + try { + await this.flushPendingOutput(pending); + if (this.pending !== pending) return; + this.completePending({ + version: GUEST_PROTOCOL_VERSION, + type: 'result', + requestId: pending.requestId, + exitCode: 124, + signal: null, + timedOut: true, + }); + this.socket?.destroy(); + } catch (error) { + this.fail(toError(error)); + } + } + private rejectPending(error: Error): void { if (!this.pending) return; - clearTimeout(this.pending.timeout); - clearTimeout(this.pending.cancellation); const pending = this.pending; + clearTimeout(pending.timeout); + clearTimeout(pending.cancellation); this.pending = undefined; + void this.finishRejectedPending(pending, error); + } + + private async finishRejectedPending(pending: PendingExecution, error: Error): Promise { + try { + await this.flushPendingOutput(pending); + } catch { + // Preserve the transport/protocol error that terminated the request. + } pending.reject(error); } diff --git a/src/microvm/workflow-command-filter.test.ts b/src/microvm/workflow-command-filter.test.ts index 97cb52b3e..5354901b8 100644 --- a/src/microvm/workflow-command-filter.test.ts +++ b/src/microvm/workflow-command-filter.test.ts @@ -50,6 +50,29 @@ describe('WorkflowCommandFilter', () => { ); }); + it('neutralizes legacy workflow syntax anywhere across chunk boundaries', () => { + const chunks = [ + Buffer.from('prefix #'), + Buffer.from('#'), + Buffer.from('[add-mask]secret\r\n'), + Buffer.from('::'), + Buffer.from('##'), + Buffer.from('[set-output name=x]owned\n'), + ]; + + expect(filter(chunks).toString()).toBe( + 'prefix [awf blocked workflow command] # #[add-mask]secret\r\n' + + '::[awf blocked workflow command] # #[set-output name=x]owned\n', + ); + }); + + it('preserves incomplete and false legacy candidates', () => { + for (const input of ['#', '##', '#x', '##x', 'ordinary # text']) { + expect(filter(Array.from(Buffer.from(input), (byte) => Buffer.from([byte]))).toString()) + .toBe(input); + } + }); + it('matches the runner whitespace trim across split UTF-8 sequences', () => { const whitespace = [ '\u0085', diff --git a/src/microvm/workflow-command-filter.ts b/src/microvm/workflow-command-filter.ts index 7180b04ab..ef7cc5420 100644 --- a/src/microvm/workflow-command-filter.ts +++ b/src/microvm/workflow-command-filter.ts @@ -1,5 +1,6 @@ const BLOCKED_COMMAND_PREFIX = Buffer.from('[awf blocked workflow command] ', 'ascii'); const NEUTRALIZED_COMMAND_START = Buffer.from(': :', 'ascii'); +const NEUTRALIZED_LEGACY_COMMAND_START = Buffer.from('# #[', 'ascii'); function isAsciiWhitespace(byte: number): boolean { return byte === 0x09 || byte === 0x0b || byte === 0x0c || byte === 0x20; @@ -52,12 +53,13 @@ function isAsciiLetter(byte: number): boolean { /** * Neutralizes GitHub Actions workflow commands in an untrusted byte stream. - * It buffers at most two candidate bytes, so arbitrary chunks and long lines + * It buffers at most four candidate bytes, so arbitrary chunks and long lines * do not require decoding or line-sized buffering. */ export class WorkflowCommandFilter { private lineCanStartCommand = true; private candidateColons = 0; + private legacyCandidate: number[] = []; private unicodeWhitespaceCandidate: number[] = []; push(data: Buffer): Buffer { @@ -74,13 +76,53 @@ export class WorkflowCommandFilter { const trailing = Buffer.from([ ...this.unicodeWhitespaceCandidate, ...Array(this.candidateColons).fill(0x3a), + ...this.legacyCandidate, ]); this.unicodeWhitespaceCandidate = []; this.candidateColons = 0; + this.legacyCandidate = []; return trailing; } private processByte(byte: number, output: number[]): void { + if (this.legacyCandidate.length === 1) { + if (byte === 0x23) { + this.legacyCandidate.push(byte); + return; + } + this.processV2Byte(0x23, output); + this.legacyCandidate = []; + } else if (this.legacyCandidate.length === 2) { + this.legacyCandidate = []; + if (byte === 0x5b) { + this.flushV2Candidate(output); + output.push(...BLOCKED_COMMAND_PREFIX, ...NEUTRALIZED_LEGACY_COMMAND_START); + this.lineCanStartCommand = false; + return; + } + this.processV2Byte(0x23, output); + this.processV2Byte(0x23, output); + } + + if (byte === 0x23) { + this.legacyCandidate.push(byte); + return; + } + this.processV2Byte(byte, output); + } + + private flushV2Candidate(output: number[]): void { + if (this.unicodeWhitespaceCandidate.length > 0) { + output.push(...this.unicodeWhitespaceCandidate); + this.unicodeWhitespaceCandidate = []; + } + if (this.candidateColons > 0) { + output.push(...Array(this.candidateColons).fill(0x3a)); + this.candidateColons = 0; + } + } + + private processV2Byte(byte: number, output: number[]): void { if (this.unicodeWhitespaceCandidate.length > 0) { this.unicodeWhitespaceCandidate.push(byte); const status = unicodeWhitespaceStatus(this.unicodeWhitespaceCandidate);