diff --git a/evidence/hosted-hardening/mutation.txt b/evidence/hosted-hardening/mutation.txt new file mode 100644 index 00000000..44bea48d --- /dev/null +++ b/evidence/hosted-hardening/mutation.txt @@ -0,0 +1,44 @@ +# Mutation 1: make the Surface entry resolve lazily again (revert fix A) +$ npx vitest run tests/hosted-hardening.test.ts -t initializes + × hosted extension hardening > resolves the Surface entry while this module initializes, before authored code can steer the loader 25ms + → expected '/private/var/folders/l7/1p99_dl16pj5x…' to be '/Users/khaliqgant/Projects/AgentWorkf…' // Object.is equality + Tests 1 failed | 10 skipped (11) +$ shasum -a 256 original restored +298a611328194a969330551de0cccdccee2221fe51d958c5e7e607d4edaf5dd2 +298a611328194a969330551de0cccdccee2221fe51d958c5e7e607d4edaf5dd2 +$ npx vitest run tests/hosted-hardening.test.ts -t initializes + ✓ tests/hosted-hardening.test.ts (11 tests | 10 skipped) 8ms + Tests 1 passed | 10 skipped (11) + +# Mutation 2: drop the hosted lock name pattern check (revert fix B) +$ npx vitest run tests/hosted-hardening.test.ts -t not one safe path component + × hosted extension hardening > refuses a hosted lock whose plugin name "../escape" is not one safe path component 29ms + → expected Error: /private/var/folders/l7/1p99_dl16p… { code: '…' } to match object { code: 'plugin_lock_invalid' } + × hosted extension hardening > refuses a hosted lock whose plugin name "a/b" is not one safe path component 6ms + → expected Error: /private/var/folders/l7/1p99_dl16p… { code: '…' } to match object { code: 'plugin_lock_invalid' } + × hosted extension hardening > refuses a hosted lock whose plugin name "." is not one safe path component 5ms + → expected Error: /private/var/folders/l7/1p99_dl16p… { code: '…' } to match object { code: 'plugin_lock_invalid' } + × hosted extension hardening > refuses a hosted lock whose plugin name ".." is not one safe path component 5ms + → expected Error: /private/var/folders/l7/1p99_dl16p… { code: '…' } to match object { code: 'plugin_lock_invalid' } + × hosted extension hardening > refuses a hosted lock whose plugin name "Babysitter" is not one safe path component 5ms + → expected Error: /private/var/folders/l7/1p99_dl16p… { code: '…' } to match object { code: 'plugin_lock_invalid' } + × hosted extension hardening > refuses a hosted lock whose plugin name "babysitter/../../etc" is not one safe path component 5ms + → expected Error: /private/var/folders/l7/1p99_dl16p… { code: '…' } to match object { code: 'plugin_lock_invalid' } +$ shasum -a 256 original restored +b1c276859dc837aecd4fb2d5578d30c0f9913ad9a72653a6c655fae2a5324646 +b1c276859dc837aecd4fb2d5578d30c0f9913ad9a72653a6c655fae2a5324646 +$ npx vitest run tests/hosted-hardening.test.ts -t not one safe path component + ✓ tests/hosted-hardening.test.ts (11 tests | 4 skipped) 36ms + Tests 7 passed | 4 skipped (11) + +# Mutation 3: restore last-match compat.base selection (revert fix C) +$ npx vitest run tests/hosted-hardening.test.ts -t first compat.base entry + × hosted extension hardening > matches the first compat.base entry for a name, not the last 17ms + → expected [Function] to not throw an error but 'Error: babysitter requires software-f…' was thrown + Tests 1 failed | 10 skipped (11) +$ shasum -a 256 original restored +3f4069271d2e16cd35dc6ce78ef2ff24067d62ca7fc1d7a0ea8bb3b53a3dd987 +3f4069271d2e16cd35dc6ce78ef2ff24067d62ca7fc1d7a0ea8bb3b53a3dd987 +$ npx vitest run tests/hosted-hardening.test.ts -t first compat.base entry + ✓ tests/hosted-hardening.test.ts (11 tests | 10 skipped) 9ms + Tests 1 passed | 10 skipped (11) diff --git a/evidence/hosted-hardening/tests.txt b/evidence/hosted-hardening/tests.txt new file mode 100644 index 00000000..4145acc6 --- /dev/null +++ b/evidence/hosted-hardening/tests.txt @@ -0,0 +1,46 @@ +$ (cd packages/sdk && npm run typecheck) +> @relayflows/sdk@2.0.32 typecheck +> tsc --noEmit && tsc -p tsconfig.type-tests.json + +Exit: 0 + +$ (cd packages/sdk && npm run build) +> tsc && node scripts/make-cli-executable.mjs + +Exit: 0 + +$ (cd packages/sdk && npm run typecheck:tests) +> @relayflows/sdk@2.0.32 typecheck:tests +> tsc -p tsconfig.tests.json + +Exit: 0 + +$ (cd packages/sdk && npx vitest run tests/hosted-hardening.test.ts tests/hosted-extension-isolation.test.ts tests/hosted-extension-protocol.test.ts tests/hosted-extension-routing.test.ts tests/hosted-base-snapshot.test.ts tests/plugin-store-bounds.test.ts tests/babysitter-native-extension.test.ts tests/flow-extension-compose.test.ts tests/plugin-extension.test.ts tests/authored-flow.test.ts) # passing lines elided + ❯ runTest node_modules/@vitest/runner/dist/index.js:1056:11 + ❯ runSuite node_modules/@vitest/runner/dist/index.js:1205:15 + ❯ runSuite node_modules/@vitest/runner/dist/index.js:1205:15 + ❯ runFiles node_modules/@vitest/runner/dist/index.js:1262:5 + ❯ startTests node_modules/@vitest/runner/dist/index.js:1271:3 + +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ +Serialized Error: { code: 'plugin_unsupported' } +This error originated in "tests/hosted-extension-protocol.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running. +The latest test that might've caused the error is "rejects two forged calls after the authoritative first outcome settles". It might mean one of the following: +- The error was thrown, while Vitest was running this test. +- If the error occurred after the test had been completed, this was the last documented test before it was thrown. +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ + + Test Files 4 failed | 6 passed (10) + Tests 34 failed | 199 passed | 47 skipped (280) + Errors 1 error + Start at 22:55:46 + Duration 51.29s (transform 11.41s, setup 783ms, collect 60.44s, tests 63.80s, environment 7ms, prepare 5.61s) + +Exit: 1 + +# Baseline: the same suites on this branch with all changes stashed (git stash push -u), macOS +$ (cd packages/sdk && npx vitest run tests/hosted-extension-isolation.test.ts -t "resolves locked artifacts") + × hosted extension capability isolation > resolves locked artifacts without importing extension top-level code + → expected [ { …(6) } ] to deeply equal [ { …(6) } ] + Tests 1 failed | 21 skipped (22) +The 34 failures in the run above are all pre-existing on this host: 33 are 'hosted extension isolation requires Linux' (these suites need Linux + bubblewrap) and the deep-equal one reproduces with the changes stashed, as captured here. CI on Linux is the authority. diff --git a/packages/sdk/src/flow-extension-compat.ts b/packages/sdk/src/flow-extension-compat.ts index ad38f3db..8d408075 100644 --- a/packages/sdk/src/flow-extension-compat.ts +++ b/packages/sdk/src/flow-extension-compat.ts @@ -42,7 +42,9 @@ function assertRuntimeRange( */ export function assertBaseCompatible(manifest: FlowExtensionManifest, base: { readonly name: string; readonly version?: string }): void { let entry: FlowExtensionManifest['compat']['base'][number] | undefined; - for (let index = 0; index < manifest.compat.base.length; index += 1) { + // First match, as `.find()` did before the intrinsic-capture rewrite: a + // later duplicate name must not widen or narrow the range that decides. + for (let index = 0; index < manifest.compat.base.length && entry === undefined; index += 1) { if (manifest.compat.base[index]!.name === base.name) entry = manifest.compat.base[index]; } if (entry === undefined) { diff --git a/packages/sdk/src/hosted-extension-declarations.ts b/packages/sdk/src/hosted-extension-declarations.ts index d3571407..b9ffa89f 100644 --- a/packages/sdk/src/hosted-extension-declarations.ts +++ b/packages/sdk/src/hosted-extension-declarations.ts @@ -67,6 +67,9 @@ const STRING_STARTS_WITH = Function.prototype.call.bind( ) as (value: string, search: string) => boolean; const LOCK_HEX64 = /^[0-9a-f]{64}$/; const LOCK_SHA = /^[0-9a-f]{40}$/; +// `name` reaches pluginStoreDirectory as a path segment, so it stays one +// lowercase kebab-case component: no separators, no `.`/`..`, no traversal. +const LOCK_NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; const LOCK_OWNER = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})$/; const LOCK_REPO = /^[A-Za-z0-9_.-]{1,100}$/; const HTTPS_GITHUB = /^https:\/\/github\.com\//; @@ -240,6 +243,7 @@ function parseHostedPluginLock(input: unknown): { ]) || entry.kind !== "flow-extension" || typeof entry.name !== "string" || + !REGEXP_TEST(LOCK_NAME, entry.name) || typeof entry.version !== "string" || typeof entry.digest !== "string" || !REGEXP_TEST(LOCK_HEX64, entry.digest) || diff --git a/packages/sdk/src/hosted-extension-sandbox.ts b/packages/sdk/src/hosted-extension-sandbox.ts index 214c4728..c7564909 100644 --- a/packages/sdk/src/hosted-extension-sandbox.ts +++ b/packages/sdk/src/hosted-extension-sandbox.ts @@ -53,6 +53,22 @@ const PATH_RESOLVE = resolve; const SPAWN = spawn; const PROCESS_EXEC_PATH = process.execPath; const PROCESS_NODE_VERSION = process.versions.node; + +/** + * The host's own `@relayflows/surface` entry, resolved once while this module + * initializes. `require.resolve` consults `Module._resolveFilename`, which is + * writable: resolving lazily would run after authored flow code has had a + * chance to replace it and could hand the sandbox a different package. The + * lookup therefore happens before any authored module can load, and a failure + * is reported when a caller actually needs the root. + */ +const SURFACE_ENTRY: string | undefined = (() => { + try { return REALPATH_SYNC(CREATE_REQUIRE(import.meta.url).resolve('@relayflows/surface')); } + catch { return undefined; } +})(); + +/** @internal Seam for the regression that pins initialization-time capture. */ +export function capturedSurfaceEntry(): string | undefined { return SURFACE_ENTRY; } const PROCESS_PLATFORM = process.platform; const PROMISE = Promise; const EVENT_ON = Function.prototype.call.bind(EventEmitter.prototype.on) as ( @@ -250,9 +266,8 @@ export async function runHostedExtensionSandbox( async function resolveSurfaceRoot(expectedVersion: string, override?: string): Promise { if (override !== undefined) return await checkedSurfaceRoot(override, expectedVersion); - let resolved: string; - try { resolved = REALPATH_SYNC(CREATE_REQUIRE(import.meta.url).resolve('@relayflows/surface')); } - catch { return unsupported('hosted extension cannot resolve @relayflows/surface'); } + const resolved = SURFACE_ENTRY; + if (resolved === undefined) return unsupported('hosted extension cannot resolve @relayflows/surface'); let directory = PATH_DIRNAME(resolved); const root = PATH_PARSE(directory).root; while (directory !== root) { diff --git a/packages/sdk/tests/hosted-hardening.test.ts b/packages/sdk/tests/hosted-hardening.test.ts new file mode 100644 index 00000000..3e15b0de --- /dev/null +++ b/packages/sdk/tests/hosted-hardening.test.ts @@ -0,0 +1,109 @@ +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { assertBaseCompatible } from '../src/flow-extension-compat.js'; +import type { FlowExtensionManifest } from '../src/flow-extension-manifest.js'; +import { capturedSurfaceEntry } from '../src/hosted-extension-sandbox.js'; +import { loadHostedExtensionArtifacts } from '../src/hosted-extension-runtime.js'; + +const roots: string[] = []; +afterEach(() => roots.splice(0).forEach(path => rmSync(path, { recursive: true, force: true }))); + +/** A project whose lockfile names one extension, with no artifact materialized. */ +function project(name: string): string { + const root = mkdtempSync(join(tmpdir(), 'hosted-hardening-')); + roots.push(root); + writeFileSync(join(root, 'package.json'), '{"type":"module"}'); + writeFileSync(join(root, 'flows.json'), JSON.stringify({ + plugins: [`github:AgentWorkforce/flows@${'a'.repeat(40)}#extensions/babysitter`], + })); + writeFileSync(join(root, 'flows.lock.json'), JSON.stringify({ + version: 2, + plugins: [{ + name, kind: 'flow-extension', version: '0.2.0', + source: { + host: 'github', owner: 'AgentWorkforce', repo: 'flows', + sha: 'a'.repeat(40), path: 'extensions/babysitter', + }, + digest: 'b'.repeat(64), manifestSha256: 'c'.repeat(64), + order: 1, resolvedAt: '2026-09-22T12:00:00.000Z', + }], + })); + writeFileSync(join(root, 'software-factory.flow.ts'), 'export default {};'); + return join(root, 'software-factory.flow.ts'); +} + +function manifest(base: FlowExtensionManifest['compat']['base']): FlowExtensionManifest { + return { + schema: 2, kind: 'flow-extension', name: 'babysitter', version: '0.2.0', + compat: { surface: '*', sdk: '*', base }, + entry: 'babysitter.flow.ts', + extends: { handlers: true, hooks: [] }, + triggers: [], + permissions: { integrations: [], harnesses: [], mcp: [], writes: [] }, + preflight: { credentials: [], servers: [] }, + } as FlowExtensionManifest; +} + +describe('hosted extension hardening', () => { + it('resolves the Surface entry while this module initializes, before authored code can steer the loader', () => { + const before = capturedSurfaceEntry(); + // A realpath: a linked workspace copy resolves outside node_modules. + expect(before).toMatch(/surface[/\\]dist[/\\]index\.js$/); + // What an authored module could do to the resolver once it is imported. + const moduleExports = createRequire(import.meta.url)('node:module') as { + _resolveFilename: (...args: unknown[]) => string; + }; + const original = moduleExports._resolveFilename; + const hostile = mkdtempSync(join(tmpdir(), 'hosted-hostile-surface-')); + roots.push(hostile); + mkdirSync(join(hostile, 'dist'), { recursive: true }); + writeFileSync(join(hostile, 'package.json'), JSON.stringify({ name: '@relayflows/surface', version: '0.0.0' })); + writeFileSync(join(hostile, 'dist', 'index.js'), 'export {};\n'); + try { + moduleExports._resolveFilename = function poisoned(request: unknown, ...rest: unknown[]): string { + if (request === '@relayflows/surface') return join(hostile, 'dist', 'index.js'); + return original.call(this, request, ...rest); + }; + // The capture already happened, so the poisoned resolver cannot be consulted. + expect(capturedSurfaceEntry()).toBe(before); + expect(capturedSurfaceEntry()).not.toContain(hostile); + } finally { + moduleExports._resolveFilename = original; + } + }); + + it.each(['../escape', 'a/b', '.', '..', 'Babysitter', 'babysitter/../../etc', ''])( + 'refuses a hosted lock whose plugin name %j is not one safe path component', + async name => { + await expect(loadHostedExtensionArtifacts(project(name))) + .rejects.toMatchObject({ code: 'plugin_lock_invalid' }); + }, + ); + + it('still accepts an ordinary kebab-case lock name, failing later on the absent artifact', async () => { + // Not plugin_lock_invalid: the name parses, and verification fails on bytes. + await expect(loadHostedExtensionArtifacts(project('babysitter'))) + .rejects.not.toMatchObject({ code: 'plugin_lock_invalid' }); + }); + + it('matches the first compat.base entry for a name, not the last', () => { + const duplicated = manifest([ + Object.freeze({ name: 'software-factory', version: '2.0.22' }), + Object.freeze({ name: 'software-factory', version: '9.9.9' }), + ]); + // First entry decides: 2.0.22 satisfies it, and the wider later duplicate cannot rescue 9.9.9. + expect(() => assertBaseCompatible(duplicated, { name: 'software-factory', version: '2.0.22' })).not.toThrow(); + expect(() => assertBaseCompatible(duplicated, { name: 'software-factory', version: '9.9.9' })) + .toThrow(expect.objectContaining({ code: 'plugin_incompatible' })); + }); + + it('keeps reporting every declared base name when none matches', () => { + expect(() => assertBaseCompatible( + manifest([Object.freeze({ name: 'software-factory', version: '*' }), Object.freeze({ name: 'garden', version: '*' })]), + { name: 'release-manager', version: '1.0.0' }, + )).toThrow(/extends software-factory, garden, not "release-manager"/); + }); +});