diff --git a/.changeset/docker-sandbox-provider.md b/.changeset/docker-sandbox-provider.md new file mode 100644 index 000000000..01dfa3f47 --- /dev/null +++ b/.changeset/docker-sandbox-provider.md @@ -0,0 +1,6 @@ +--- +'@truefoundry/trueforge-core': patch +'@truefoundry/trueforge': patch +--- + +Add a Docker sandbox provider with optional GPU passthrough, widen the sandbox provider manifest to a discriminated union, and let a provider declare whether it supports Code Mode so sessions degrade instead of failing. diff --git a/packages/trueforge-core/src/core/sandbox/Sandbox.ts b/packages/trueforge-core/src/core/sandbox/Sandbox.ts index 2a8b0a774..1867e49be 100644 --- a/packages/trueforge-core/src/core/sandbox/Sandbox.ts +++ b/packages/trueforge-core/src/core/sandbox/Sandbox.ts @@ -267,6 +267,14 @@ export class Sandbox extends LocalToolMCP { if (!servers.length) { return; } + if (this.provider.supportsCodeMode === false) { + // Degrade rather than fail: the agent keeps its tools and its sandbox, and + // simply routes calls individually instead of batching them in a script. + this.logger.info('Code Mode unavailable for this sandbox provider; continuing without it', { + provider: this.provider.type, + }); + return; + } if (this.codeModeDispatcher !== undefined || this.codeModeTransport !== undefined) { throw new Error('Code Mode is already configured for this Sandbox'); } diff --git a/packages/trueforge-core/src/core/sandbox/provider/Provider.ts b/packages/trueforge-core/src/core/sandbox/provider/Provider.ts index 6801c239d..08ca54bfd 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/Provider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/Provider.ts @@ -67,6 +67,15 @@ export interface SandboxBuild { export interface SandboxProvider { /** Stable provider kind used in fancy sandbox ids and carry-forward (plain string). */ readonly type: string; + /** + * Whether this provider can carry Code Mode's bidirectional transport. + * + * Optional for backwards compatibility: absent means yes, which is correct for + * every provider that predates the flag. A provider that sets this to false is + * skipped rather than asked and allowed to throw -- Code Mode is an + * optimisation, and losing it must not fail the session. + */ + readonly supportsCodeMode?: boolean; /** * Ensures the release image is being built into the provider's backing store and * returns its current status. Idempotent: an already-built image reports `ready`; diff --git a/packages/trueforge/catalog/sandbox-catalog.yaml b/packages/trueforge/catalog/sandbox-catalog.yaml index fc80d83aa..02dccc1dc 100644 --- a/packages/trueforge/catalog/sandbox-catalog.yaml +++ b/packages/trueforge/catalog/sandbox-catalog.yaml @@ -7,3 +7,10 @@ providers: auto_stop_interval_in_minutes: 5 auto_archive_interval_in_minutes: 60 auto_delete_interval_in_minutes: 7200 + - type: docker + # The image must provide python3 and pydantic: the sandbox bootstrap runs a + # Python script to materialise git-backed skills, and without them + # initialisation fails with "python3: not found" while the session still + # starts, which makes the failure easy to miss. + image: python:3.12-slim + exec_timeout_ms: 600000 diff --git a/packages/trueforge/jest.docker-sandbox.contract.config.cjs b/packages/trueforge/jest.docker-sandbox.contract.config.cjs new file mode 100644 index 000000000..6763d8564 --- /dev/null +++ b/packages/trueforge/jest.docker-sandbox.contract.config.cjs @@ -0,0 +1,43 @@ +/** @type {import('jest').Config} */ +module.exports = { + testEnvironment: 'node', + transform: { + '^.+\\.tsx?$': [ + '@swc/jest', + { + jsc: { + parser: { syntax: 'typescript', decorators: true, dynamicImport: true }, + target: 'es2022', + }, + module: { type: 'commonjs' }, + }, + ], + '^.+\\.m?js$': [ + '@swc/jest', + { + jsc: { + parser: { syntax: 'ecmascript', dynamicImport: true }, + target: 'es2022', + }, + module: { type: 'commonjs' }, + }, + ], + }, + transformIgnorePatterns: [], + moduleNameMapper: { + '^(\\.{1,2}/.*)\\.js$': '$1', + '^@truefoundry/trueforge-core/agent-session$': '/../trueforge-core/src/agent-session/index.ts', + '^@truefoundry/trueforge-core/agent-session/(.*)$': '/../trueforge-core/src/agent-session/$1', + '^@truefoundry/trueforge-core/request-reply$': '/../trueforge-core/src/request-reply/index.ts', + '^@truefoundry/trueforge-core/request-reply/(.*)$': '/../trueforge-core/src/request-reply/$1', + '^@truefoundry/trueforge-core/core$': '/../trueforge-core/src/core/index.ts', + '^@truefoundry/trueforge-core/core/(.*)$': '/../trueforge-core/src/core/$1', + }, + testTimeout: 120_000, + maxWorkers: 1, + roots: ['/tests/unit'], + testMatch: ['/tests/unit/sandbox/docker/**/*.contract.test.ts'], + // The GPU suite is separate: it pulls a multi-gigabyte CUDA image, which is not + // something the plain contract run should do on a machine that has no GPU to use it. + testPathIgnorePatterns: ['/node_modules/', 'gpu\\.contract\\.test\\.ts$'], +}; diff --git a/packages/trueforge/jest.docker-sandbox.gpu.config.cjs b/packages/trueforge/jest.docker-sandbox.gpu.config.cjs new file mode 100644 index 000000000..4330adcfd --- /dev/null +++ b/packages/trueforge/jest.docker-sandbox.gpu.config.cjs @@ -0,0 +1,40 @@ +/** @type {import('jest').Config} */ +module.exports = { + testEnvironment: 'node', + transform: { + '^.+\\.tsx?$': [ + '@swc/jest', + { + jsc: { + parser: { syntax: 'typescript', decorators: true, dynamicImport: true }, + target: 'es2022', + }, + module: { type: 'commonjs' }, + }, + ], + '^.+\\.m?js$': [ + '@swc/jest', + { + jsc: { + parser: { syntax: 'ecmascript', dynamicImport: true }, + target: 'es2022', + }, + module: { type: 'commonjs' }, + }, + ], + }, + transformIgnorePatterns: [], + moduleNameMapper: { + '^(\\.{1,2}/.*)\\.js$': '$1', + '^@truefoundry/trueforge-core/agent-session$': '/../trueforge-core/src/agent-session/index.ts', + '^@truefoundry/trueforge-core/agent-session/(.*)$': '/../trueforge-core/src/agent-session/$1', + '^@truefoundry/trueforge-core/request-reply$': '/../trueforge-core/src/request-reply/index.ts', + '^@truefoundry/trueforge-core/request-reply/(.*)$': '/../trueforge-core/src/request-reply/$1', + '^@truefoundry/trueforge-core/core$': '/../trueforge-core/src/core/index.ts', + '^@truefoundry/trueforge-core/core/(.*)$': '/../trueforge-core/src/core/$1', + }, + testTimeout: 120_000, + maxWorkers: 1, + roots: ['/tests/unit'], + testMatch: ['/tests/unit/sandbox/docker/**/gpu.contract.test.ts'], +}; diff --git a/packages/trueforge/package.json b/packages/trueforge/package.json index 1790bf5da..763f964ff 100644 --- a/packages/trueforge/package.json +++ b/packages/trueforge/package.json @@ -49,6 +49,8 @@ "test:store:sqlite": "jest --config jest.store.sqlite.config.cjs", "test": "pnpm run build:gen && NODE_OPTIONS='--conditions=trueforge-dev' node --env-file=.env.test ./node_modules/jest/bin/jest.js --config jest.unit.config.cjs", "test:local-sandbox:contract": "pnpm run build:gen && NODE_OPTIONS='--conditions=trueforge-dev' node --env-file=.env.test ./node_modules/jest/bin/jest.js --config jest.local-sandbox.contract.config.cjs", + "test:docker-sandbox:contract": "pnpm run build:gen && NODE_OPTIONS='--conditions=trueforge-dev' node --env-file=.env.test ./node_modules/jest/bin/jest.js --config jest.docker-sandbox.contract.config.cjs", + "test:docker-sandbox:gpu": "pnpm run build:gen && NODE_OPTIONS='--conditions=trueforge-dev' node --env-file=.env.test ./node_modules/jest/bin/jest.js --config jest.docker-sandbox.gpu.config.cjs", "smoke:local-sandbox": "pnpm run build:gen && NODE_OPTIONS='--conditions=trueforge-dev' jest --config jest.local-sandbox.smoke.config.cjs --runInBand --forceExit tests/sandbox/local/smoke.test.ts", "smoke:local-sandbox:lima": "bash scripts/local-sandbox/smoke-lima.sh", "probe:loopback": "pnpm exec tsx scripts/local-sandbox/probe-loopback.ts", diff --git a/packages/trueforge/src/apis/sandboxProviders.ts b/packages/trueforge/src/apis/sandboxProviders.ts index 9df3afa23..4f031688e 100644 --- a/packages/trueforge/src/apis/sandboxProviders.ts +++ b/packages/trueforge/src/apis/sandboxProviders.ts @@ -4,12 +4,7 @@ import type { Logger } from 'winston'; import type { ISandboxProviderStore, SandboxProviderRecord } from '../db/sandboxProviderStore'; import type { WithTransaction } from '../db/transaction'; import { getSandboxProviderRoute, putSandboxProviderRoute } from '../routes/sandboxProviderRoutes'; -import { - checkSnapshotStatus, - isDaytonaAuthError, - toDaytonaSandboxProvider, - toSandboxStatus, -} from '../sandbox/providerUtils'; +import { checkSnapshotStatus, isDaytonaAuthError, toSandboxProvider, toSandboxStatus } from '../sandbox/providerUtils'; import type { SandboxProviderManifest, UpdateSandboxProviderRequest } from '../schemas/sandboxProvider'; import { MissingStoredSecretError, resolveStoredSecretValue, toRedactedSecretValue } from '../utils/secretRedaction'; import { TENANT_ID } from './sessions'; @@ -24,10 +19,18 @@ export interface SandboxProvidersRouterDeps { } function redactSandboxProvider(manifest: SandboxProviderManifest): SandboxProviderManifest { - return { - ...manifest, - auth: { api_key: toRedactedSecretValue(manifest.auth.api_key) }, - }; + // Switch rather than an optional-chain on `auth`: a new variant that does carry + // credentials should fail to compile here instead of silently returning them. + switch (manifest.type) { + case 'daytona': + return { + ...manifest, + auth: { api_key: toRedactedSecretValue(manifest.auth.api_key) }, + }; + case 'docker': + // No credentials: the container runtime is a local socket. + return manifest; + } } /** Admin/settings sandbox provider surface (mounted at /api/v1/settings/sandbox-providers). */ @@ -58,15 +61,24 @@ export function createSandboxProvidersRouter(deps: SandboxProvider const putHandler: RouteHandler = async c => { const body: UpdateSandboxProviderRequest = c.req.valid('json'); const incoming = body.manifest; - const resolveManifest = (existing: SandboxProviderRecord | undefined): SandboxProviderManifest => ({ - ...incoming, - auth: { - api_key: resolveStoredSecretValue({ - incoming: incoming.auth.api_key, - existing: existing?.manifest.auth.api_key, - }), - }, - }); + const resolveManifest = (existing: SandboxProviderRecord | undefined): SandboxProviderManifest => { + if (incoming.type !== 'daytona') { + // No stored secret to carry forward: the container backend has no auth. + return incoming; + } + const existingManifest = existing?.manifest; + return { + ...incoming, + auth: { + api_key: resolveStoredSecretValue({ + incoming: incoming.auth.api_key, + // Only a stored daytona manifest can supply the previous key. If the + // tenant is switching backends, there is nothing to carry forward. + existing: existingManifest?.type === 'daytona' ? existingManifest.auth.api_key : undefined, + }), + }, + }; + }; try { // NOTE: build (Daytona network I/O) runs inside the transaction for now; the design is being revisited. const { manifest, status } = await deps.withTransaction(async transaction => { @@ -74,7 +86,7 @@ export function createSandboxProvidersRouter(deps: SandboxProvider const resolved = resolveManifest(locked); // Pass persisted build_metadata so a settings re-save does not start a new snapshot for a // bumped SANDBOX_IMAGE_URI (upgrades are unsupported — first configure has no metadata). - const provider = toDaytonaSandboxProvider({ + const provider = toSandboxProvider({ manifest: resolved, tenant_id: TENANT_ID, logger: deps.logger, diff --git a/packages/trueforge/src/runtime/sessionResources.ts b/packages/trueforge/src/runtime/sessionResources.ts index e053e6694..2e4471032 100644 --- a/packages/trueforge/src/runtime/sessionResources.ts +++ b/packages/trueforge/src/runtime/sessionResources.ts @@ -25,7 +25,7 @@ import { isMcpAuthRequired, resolveMcpAuth } from '../mcp/auth/mcpDcr'; import type { IOAuthTokenStore } from '../mcp/auth/types'; import { LocalSandboxProvider } from '../sandbox/local/provider/LocalSandboxProvider'; import { getCachedLocalSandboxSupport, isLocalSandboxFallbackEnabled } from '../sandbox/localRuntime'; -import { toDaytonaSandboxProvider } from '../sandbox/providerUtils'; +import { toSandboxProvider } from '../sandbox/providerUtils'; import { resolveConfiguredMcpRequestHeaders } from '../schemas/mcpServer'; export interface McpConnection { @@ -238,7 +238,7 @@ export async function resolveSandboxProvider({ if (record !== undefined) { // Clone from the snapshot that was actually built (persisted build_ref), not a name // derived from the current image — otherwise an image bump breaks creation until rebuild. - return toDaytonaSandboxProvider({ + return toSandboxProvider({ manifest: record.manifest, tenant_id, logger, diff --git a/packages/trueforge/src/sandbox/docker/provider/DockerSandboxProvider.ts b/packages/trueforge/src/sandbox/docker/provider/DockerSandboxProvider.ts new file mode 100644 index 000000000..fe869fadc --- /dev/null +++ b/packages/trueforge/src/sandbox/docker/provider/DockerSandboxProvider.ts @@ -0,0 +1,796 @@ +/** + * Container-backed SandboxProvider. + * + * One container per sandbox, addressed by the absolute working directory inside + * it. That choice matters for two reasons: + * + * - The provider contract requires `pwd` to print the sandbox id, so the id has + * to *be* a path rather than a container name. + * - An absolute id activates the path-id branch of the shared contract suite, + * which asserts that one sandbox cannot reach a sibling by `../` or by + * absolute path. Separate containers satisfy that structurally: the sibling + * path does not exist in the other mount namespace. + * + * The sandbox is an image, which is the point: a workload that needs a specific + * CUDA or Python toolchain gets a reproducible one, which a host-process sandbox + * cannot offer. `gpus` maps onto `--gpus` so the container toolkit does the device + * and driver plumbing. + * + * File transfer goes through `docker exec` with a piped stdin/stdout rather than + * encoding payloads into a command string. Encoding into argv caps out around + * 96 KiB on `MAX_ARG_STRLEN` / `E2BIG` (see upstream issue #416), and a sandbox + * that cannot accept a file larger than a small source file is not much use. + */ + +import type { + CodeModeTransport, + ExecResult, + SandboxBuild, + SandboxExecParams, + SandboxProvider, +} from '@truefoundry/trueforge-core/core'; +import { + absolutizeRelativeExecEnv, + SandboxFileNotFoundError, + SandboxFileTooLargeError, + SandboxNotAvailableError, + SandboxPathIsDirectoryError, + shellEscape, + validateNoPathTraversal, +} from '@truefoundry/trueforge-core/core'; +import { spawn } from 'node:child_process'; +import { posix, resolve, sep } from 'node:path'; +import { ulid } from 'ulid'; +import type { Logger } from 'winston'; + +/** Parent directory of every sandbox root inside the container. */ +const SANDBOX_PARENT = '/sandbox'; +const DEFAULT_EXEC_TIMEOUT_SECONDS = 60; +const DEFAULT_FILE_MAX_BYTES = 10 * 1024 * 1024; +/** Cap for `docker version` / `docker image inspect` probes, not general exec. */ +const PROBE_TIMEOUT_MS = 10_000; +const CONTAINER_NAME_PREFIX = 'tfy-sbx-'; +/** Slack between the in-container timeout and the outer client kill. */ +const OUTER_TIMEOUT_GRACE_MS = 10_000; +/** Marks containers this provider owns, so stale ones can be found and reaped. */ +const OWNER_LABEL = 'com.truefoundry.trueforge.sandbox'; +/** + * Narrows ownership below "any TrueForge sandbox on this daemon". + * + * Without it a reaper can only operate globally, which makes it a footgun rather + * than a tool: a second server, another developer, or a test run sharing the + * daemon would have its live sandboxes deleted. Every reap is scoped, and a scope + * is required rather than defaulted at the call site. + */ +const SCOPE_LABEL = 'com.truefoundry.trueforge.sandbox.scope'; +const DEFAULT_SCOPE = 'default'; +/** Sandbox ids are ULIDs lowercased; anything else is not one of ours. */ +const SANDBOX_SLUG_RE = /^[0-9a-z]{26}$/; + +/** + * In-flight and failed image pulls, keyed by image reference. + * + * Deliberately module-scoped rather than per-instance: the server constructs a + * fresh provider for every turn (see `resolveSandboxProvider`), so instance state + * would make a pull started by the settings PUT invisible to the next caller, + * which would then start a second pull and never observe the first one's failure. + * Process-scoped is the correct lifetime for "is this image being fetched". + */ +const pullState = new Map | undefined; failure: string | undefined }>(); + +export interface DockerSandboxProviderOptions { + /** Image the sandbox runs. Must provide a POSIX shell and `python3`. */ + image: string; + logger: Logger; + /** `docker` by default; set to `podman` or an absolute path to override. */ + dockerBinary?: string; + /** + * Value passed to `--gpus`, e.g. `all` or `device=0`. Omitted means no GPU, + * which is the right default: attaching a GPU to a sandbox that does not need + * one wastes a scarce device and slows container start. + */ + gpus?: string; + /** + * Ownership scope for containers this provider creates. Only a reaper given the + * same scope will remove them, so two servers (or a test run and a dev server) + * sharing one daemon cannot delete each other's live sandboxes. + */ + scope?: string; + execTimeoutSeconds?: number; + fileMaxBytesForDownload?: number; + /** + * Extra `docker run` arguments. Intended for read-only host mounts such as a + * CUDA toolkit, so the image can stay small. Never interpolated through a + * shell. + */ + extraRunArgs?: readonly string[]; +} + +export type DockerSandboxSupportResult = { supported: true; version: string } | { supported: false; reason: string }; + +interface RunResult { + exitCode: number; + stdout: Buffer; + stderr: string; + timedOut: boolean; + /** + * The process was killed because stdout passed `maxStdoutBytes`. Distinct from + * an exit code: the child is killed mid-stream, so its status says nothing about + * why. Without this flag an oversized download is indistinguishable from a + * missing file. + */ + outputCapExceeded: boolean; +} + +/** + * Thrown when Code Mode is requested on a provider that has no transport for it. + * Typed so a caller can detect it and decline the capability, rather than having + * to string-match a generic Error at the point the agent already committed. + */ +export class CodeModeUnsupportedError extends Error { + readonly providerType: string; + + constructor(providerType: string) { + super(`sandbox provider "${providerType}" does not support Code Mode`); + this.name = 'CodeModeUnsupportedError'; + this.providerType = providerType; + } +} + +export class DockerSandboxProvider implements SandboxProvider { + readonly type = 'docker'; + /** + * Code Mode needs a bidirectional transport between harness and sandbox. The + * local provider uses a unix socket on a shared filesystem, which a container + * does not have by construction. Declared false so the session degrades to + * ordinary tool calls instead of constructing a transport that throws. + */ + readonly supportsCodeMode = false; + + private readonly image: string; + private readonly dockerBinary: string; + private readonly gpus: string | undefined; + private readonly execTimeoutSeconds: number; + private readonly fileMaxBytesForDownload: number; + private readonly extraRunArgs: readonly string[]; + private readonly scope: string; + private readonly logger: Logger; + + /** + * Sandbox ids created by *this* instance, for `dispose()` only. It is not a + * lookup table: the container name is derived from the sandbox id, so a + * provider built in a later turn can still address a sandbox it did not create. + */ + private readonly ownCreations = new Set(); + + private static readonly readyBuild: SandboxBuild = { status: 'ready', reason: null, metadata: null }; + + constructor(options: DockerSandboxProviderOptions) { + this.image = options.image; + this.dockerBinary = options.dockerBinary ?? 'docker'; + this.gpus = options.gpus; + this.execTimeoutSeconds = options.execTimeoutSeconds ?? DEFAULT_EXEC_TIMEOUT_SECONDS; + this.fileMaxBytesForDownload = options.fileMaxBytesForDownload ?? DEFAULT_FILE_MAX_BYTES; + this.extraRunArgs = options.extraRunArgs ?? []; + this.scope = options.scope ?? DEFAULT_SCOPE; + this.logger = options.logger; + } + + /** Probe whether a usable container runtime is present. */ + static async isSupported(dockerBinary = 'docker'): Promise { + try { + const result = await runProcess({ + file: dockerBinary, + args: ['version', '--format', '{{.Server.Version}}'], + timeoutMs: PROBE_TIMEOUT_MS, + }); + if (result.exitCode !== 0) { + return { + supported: false, + reason: `\`${dockerBinary} version\` exited ${String(result.exitCode)}: ${result.stderr.trim() || 'no stderr'}`, + }; + } + return { supported: true, version: result.stdout.toString('utf8').trim() }; + } catch (error) { + return { supported: false, reason: `${dockerBinary} not usable: ${errorMessage(error)}` }; + } + } + + /** + * Ensures the image is present, pulling in the background if not. + * + * Must return promptly: callers wrap this in a short `withTimeout` (3s in the + * settings route), and a cold CUDA image is several gigabytes. So the pull is + * started detached and the call reports `pending`, which is exactly the + * contract the interface documents. + */ + async buildImage(): Promise { + if (await this.imagePresent()) { + return DockerSandboxProvider.readyBuild; + } + const state = pullState.get(this.image) ?? { inFlight: undefined, failure: undefined }; + if (state.inFlight === undefined) { + state.failure = undefined; + state.inFlight = runProcess({ + file: this.dockerBinary, + args: ['pull', this.image], + timeoutMs: 60 * 60_000, + }) + .then(pull => { + if (pull.exitCode !== 0) { + state.failure = pull.stderr.trim() || `exit ${String(pull.exitCode)}`; + } + }) + .catch((error: unknown) => { + state.failure = errorMessage(error); + }) + .finally(() => { + state.inFlight = undefined; + }); + pullState.set(this.image, state); + this.logger.info('DockerSandboxProvider started image pull', { image: this.image }); + } + return { status: 'pending', reason: `pulling ${this.image}`, metadata: { image: this.image } }; + } + + async getImageBuildStatus(): Promise { + if (await this.imagePresent()) { + return DockerSandboxProvider.readyBuild; + } + const state = pullState.get(this.image); + if (state?.inFlight !== undefined) { + return { status: 'pending', reason: `pulling ${this.image}`, metadata: { image: this.image } }; + } + if (state?.failure !== undefined) { + return { + status: 'failed', + reason: `failed to pull ${this.image}: ${state.failure}`, + metadata: { image: this.image }, + }; + } + return { status: 'pending', reason: `image ${this.image} not present locally`, metadata: { image: this.image } }; + } + + private async imagePresent(): Promise { + const result = await runProcess({ + file: this.dockerBinary, + args: ['image', 'inspect', this.image], + timeoutMs: PROBE_TIMEOUT_MS, + }).catch(() => undefined); + return result?.exitCode === 0; + } + + async createSandbox(): Promise<{ sandboxId: string }> { + const id = ulid().toLowerCase(); + const containerName = `${CONTAINER_NAME_PREFIX}${id}`; + const sandboxId = posix.join(SANDBOX_PARENT, id); + + const args = [ + 'run', + '--detach', + '--name', + containerName, + // Keep the container alive without a workload; every command arrives via + // `docker exec`. `sleep infinity` as PID 1 reaps nothing, so init is on. + '--init', + '--workdir', + sandboxId, + // Ownership marker so stale sandboxes can be found and reaped without + // relying on any process having kept a handle to them. + '--label', + `${OWNER_LABEL}=1`, + '--label', + `${SCOPE_LABEL}=${this.scope}`, + ...(this.gpus === undefined ? [] : ['--gpus', this.gpus]), + ...this.extraRunArgs, + this.image, + 'sleep', + 'infinity', + ]; + + const created = await runProcess({ file: this.dockerBinary, args, timeoutMs: 5 * 60_000 }); + if (created.exitCode !== 0) { + throw new SandboxNotAvailableError(`failed to start sandbox container: ${created.stderr.trim() || 'no stderr'}`); + } + + this.ownCreations.add(sandboxId); + + // `--workdir` creates the directory, but the layout subdirectories and the + // venv do not exist yet. Failure here must not leak the container. + try { + await this.execInContainer({ + containerName, + command: [ + `mkdir -p ${shellEscape(this.getToolResultDumpDir())}`, + shellEscape(this.getFileUploadsDir()), + shellEscape(this.getSkillsDir()), + ].join(' '), + cwd: sandboxId, + timeoutSeconds: this.execTimeoutSeconds, + }); + } catch (error) { + await this.removeContainer(sandboxId).catch(() => undefined); + throw error; + } + + this.logger.info('DockerSandboxProvider created sandbox', { + sandboxId, + containerName, + image: this.image, + gpus: this.gpus ?? null, + }); + return { sandboxId }; + } + + async exec(params: SandboxExecParams): Promise { + const containerName = this.requireContainer(params.sandboxId); + try { + const cwd = + params.cwd === undefined || params.cwd === '' + ? params.sandboxId + : this.resolveInSandboxRoot(params.sandboxId, params.cwd); + const env = + params.env === undefined ? undefined : absolutizeRelativeExecEnv({ root: params.sandboxId, env: params.env }); + + const result = await this.execInContainer({ + containerName, + command: params.command, + cwd, + ...(env === undefined ? {} : { env }), + timeoutSeconds: params.timeoutSeconds ?? this.execTimeoutSeconds, + }); + + return { + success: true, + response: { + exitCode: result.exitCode, + result: result.stdout.toString('utf8') + result.stderr, + }, + }; + } catch (error) { + if (error instanceof SandboxNotAvailableError) { + throw error; + } + return { success: false, error: errorMessage(error) }; + } + } + + getAdditionalInstructions(): string { + return [ + 'SANDBOX RULES:', + `- Commands run inside a container from image ${this.image}.`, + ...(this.gpus === undefined ? [] : ['- An NVIDIA GPU is attached. `nvidia-smi` and CUDA are available.']), + '- uploads, skills, and tool-results live in the sandbox working directory.', + '- ALL file creation and writes MUST stay within the sandbox working directory.', + '- The container is discarded when the sandbox ends; nothing outside the working directory persists.', + ].join('\n'); + } + + // Cwd-relative, matching the local provider: exec cwd is the sandbox root, so + // the layout paths stay free of absolute prefixes. + getToolResultDumpDir(): string { + return 'tool-results'; + } + + getGitCredentialsPath(): string { + return '.git-credentials'; + } + + getFileUploadsDir(): string { + return 'uploads'; + } + + getSkillsDir(): string { + return 'skills'; + } + + getGitDownloaderPath(): string { + return 'git_downloader.py'; + } + + async downloadFile(params: { sandboxId: string; path: string }): Promise { + const containerName = this.requireContainer(params.sandboxId); + const absolutePath = this.resolveInSandboxRoot(params.sandboxId, params.path); + + // Classification and read happen in one invocation. Two `docker exec` calls + // would leave a window in which the path could be swapped for a symlink + // between the check and the read. Distinct exit codes carry the error kind + // back, so stdout stays pure file bytes. + const read = await this.execInContainer({ + containerName, + command: containedCommand({ + root: params.sandboxId, + path: absolutePath, + timeoutSeconds: this.execTimeoutSeconds, + command: [ + 'if [ -d "$__tfy_target" ]; then exit 78; fi', + 'if [ ! -f "$__tfy_target" ]; then exit 79; fi', + `if [ "$(wc -c < "$__tfy_target")" -gt ${String(this.fileMaxBytesForDownload)} ]; then exit 80; fi`, + 'cat "$__tfy_target"', + ].join('; '), + }), + cwd: params.sandboxId, + timeoutSeconds: this.execTimeoutSeconds, + maxStdoutBytes: this.fileMaxBytesForDownload, + raw: true, + }); + + // The in-shell `wc -c` check and the `cat` are one invocation, but a sandbox + // process can still grow the file mid-stream, so the streaming cap is the + // final enforcement layer and has to report itself as such. + if (read.outputCapExceeded) { + throw new SandboxFileTooLargeError(params.path, this.fileMaxBytesForDownload + 1, this.fileMaxBytesForDownload); + } + + switch (read.exitCode) { + case 0: + return read.stdout; + case 78: + throw new SandboxPathIsDirectoryError(params.path); + case 80: + throw new SandboxFileTooLargeError(params.path, this.fileMaxBytesForDownload + 1, this.fileMaxBytesForDownload); + case CONTAINMENT_VIOLATION_EXIT: + case 79: + default: + throw new SandboxFileNotFoundError(params.path); + } + } + + async uploadFile(params: { sandboxId: string; remotePath: string; content: Buffer }): Promise { + const containerName = this.requireContainer(params.sandboxId); + const absolutePath = this.resolveInSandboxRoot(params.sandboxId, params.remotePath); + + // Payload travels on stdin. Encoding it into the command string would cap + // uploads at roughly 96 KiB (MAX_ARG_STRLEN); see upstream issue #416. + const result = await this.execInContainer({ + containerName, + command: containedCommand({ + root: params.sandboxId, + path: absolutePath, + timeoutSeconds: this.execTimeoutSeconds, + command: 'mkdir -p "$(dirname "$__tfy_target")" && cat > "$__tfy_target"', + }), + cwd: params.sandboxId, + timeoutSeconds: this.execTimeoutSeconds, + stdin: params.content, + raw: true, + }); + if (result.exitCode === CONTAINMENT_VIOLATION_EXIT) { + throw new SandboxFileNotFoundError(params.remotePath); + } + if (result.exitCode !== 0) { + throw new Error(`upload to ${params.remotePath} failed: ${result.stderr.trim() || 'no stderr'}`); + } + } + + /** + * Code Mode needs a bidirectional transport between the harness and the + * sandbox. The local provider uses a unix socket on a shared filesystem, which + * a container does not have by construction. Wiring this up needs a deliberate + * transport choice, so it throws rather than half-working. + */ + createCodeModeTransport(): CodeModeTransport { + throw new CodeModeUnsupportedError(this.type); + } + + /** Removes containers this instance started. Safe to call twice. */ + async dispose(): Promise { + const ids = [...this.ownCreations]; + await Promise.all(ids.map(async id => this.removeContainer(id).catch(() => undefined))); + } + + /** + * Removes sandbox containers created more than `olderThanMs` ago. + * + * `dispose()` alone is not enough: the server builds a fresh provider per turn + * and never disposes it, so nothing would ever clean up. Ownership is recovered + * from the container label rather than in-process bookkeeping, which also + * reclaims sandboxes orphaned by a server restart or crash. + */ + static async reapStale(params: { + /** Only containers created with this scope are eligible. */ + scope: string; + olderThanMs: number; + logger: Logger; + dockerBinary?: string; + }): Promise<{ removed: string[] }> { + if (!Number.isFinite(params.olderThanMs) || params.olderThanMs < 0) { + // A negative cutoff would make every container in scope stale, including + // ones a live session is using. Callers that want that must say so by + // passing 0, which is at least explicit about meaning "everything". + throw new RangeError(`olderThanMs must be a non-negative number, got ${String(params.olderThanMs)}`); + } + const docker = params.dockerBinary ?? 'docker'; + const listed = await runProcess({ + file: docker, + args: [ + 'ps', + '--all', + '--filter', + `label=${OWNER_LABEL}`, + '--filter', + `label=${SCOPE_LABEL}=${params.scope}`, + '--format', + '{{.Names}}', + ], + timeoutMs: PROBE_TIMEOUT_MS, + }).catch(() => undefined); + if (listed?.exitCode !== 0) { + return { removed: [] }; + } + const names = listed.stdout + .toString('utf8') + .split('\n') + .map(line => line.trim()) + .filter(line => line.length > 0); + if (names.length === 0) { + return { removed: [] }; + } + + // `docker ps --format {{.CreatedAt}}` emits e.g. "2026-08-25 23:59:01 +0530 IST", + // which Date.parse rejects as NaN. `inspect .Created` is RFC 3339 instead. + const inspected = await runProcess({ + file: docker, + args: ['inspect', '--format', '{{.Name}}\t{{.Created}}', ...names], + timeoutMs: PROBE_TIMEOUT_MS, + }).catch(() => undefined); + if (inspected?.exitCode !== 0) { + return { removed: [] }; + } + + const cutoff = Date.now() - params.olderThanMs; + const stale: string[] = []; + for (const line of inspected.stdout.toString('utf8').split('\n')) { + const [rawName, createdAt] = line.split('\t'); + if (rawName === undefined || createdAt === undefined) { + continue; + } + // `.Name` comes back with a leading slash. + const name = rawName.trim().replace(/^\//, ''); + if (name.length === 0) { + continue; + } + const created = Date.parse(createdAt.trim()); + // An unparseable timestamp must not be read as "ancient" — skip it rather + // than delete a container that might be in use. + if (Number.isNaN(created) || created >= cutoff) { + continue; + } + stale.push(name); + } + + await Promise.all( + stale.map(async name => + runProcess({ + file: docker, + args: ['rm', '--force', '--volumes', name], + timeoutMs: 60_000, + }).catch(() => undefined), + ), + ); + if (stale.length > 0) { + params.logger.info('DockerSandboxProvider reaped stale sandboxes', { + count: stale.length, + scope: params.scope, + }); + } + return { removed: stale }; + } + + private async removeContainer(sandboxId: string): Promise { + this.ownCreations.delete(sandboxId); + await runProcess({ + file: this.dockerBinary, + args: ['rm', '--force', '--volumes', containerNameFor(sandboxId)], + timeoutMs: 60_000, + }); + } + + /** + * Container name for a sandbox id, derived rather than looked up. + * + * The server constructs a new provider for every turn and then hands it a + * sandbox id carried over from a previous turn, so any in-instance map would be + * empty exactly when it was needed. Deriving the name makes a sandbox + * addressable by any provider instance pointed at the same daemon. + */ + private requireContainer(sandboxId: string): string { + return containerNameFor(sandboxId); + } + + /** + * Confine a caller-supplied path to the sandbox root. Uses the platform + * resolver for `..` collapsing, then re-checks containment, because a path + * that escapes must be reported as not-found rather than silently clamped. + */ + private resolveInSandboxRoot(sandboxRootPath: string, userPath: string): string { + validateNoPathTraversal(userPath); + const resolved = userPath.startsWith('/') ? resolve(userPath) : resolve(sandboxRootPath, userPath); + const root = resolve(sandboxRootPath); + if (resolved !== root && !resolved.startsWith(root + sep)) { + throw new SandboxFileNotFoundError(userPath); + } + return resolved; + } + + private async execInContainer(params: { + containerName: string; + command: string; + cwd: string; + env?: Record; + timeoutSeconds: number; + stdin?: Buffer; + maxStdoutBytes?: number; + /** Command already carries its own in-container `timeout`; do not add one. */ + raw?: boolean; + }): Promise { + const args = ['exec', '--workdir', params.cwd]; + if (params.stdin !== undefined) { + args.push('--interactive'); + } + for (const [key, value] of Object.entries(params.env ?? {})) { + args.push('--env', `${key}=${value}`); + } + // Killing the local `docker exec` client does not kill the process inside the + // container (runc#3359), so the bound has to be applied in there too. The + // outer kill below remains as a backstop for a wedged daemon connection. + // `--verbose` makes the timeout self-describing on stderr ("timeout: sending + // signal TERM to command ..."). Needed because GNU timeout exits 124 when it + // fires *and* passes a command's own 124 straight through, so the status alone + // cannot distinguish them. + const command = + params.raw === true + ? params.command + : `timeout --verbose ${String(params.timeoutSeconds)}s sh -c ${shellEscape(params.command)}`; + args.push(params.containerName, 'sh', '-c', command); + + const result = await runProcess({ + file: this.dockerBinary, + args, + // Grace margin so the in-container `timeout` fires first and the workload is + // actually killed. If the outer bound won the race we would kill the client + // and leave the command running. + timeoutMs: params.timeoutSeconds * 1000 + OUTER_TIMEOUT_GRACE_MS, + ...(params.stdin === undefined ? {} : { stdin: params.stdin }), + ...(params.maxStdoutBytes === undefined ? {} : { maxStdoutBytes: params.maxStdoutBytes }), + }); + if (result.timedOut) { + throw new Error( + `command exceeded ${String(params.timeoutSeconds)}s and the container-side timeout did not fire; ` + + 'the docker daemon connection may be wedged', + ); + } + // Exit 124 is deliberately *not* turned into a failure. The contract puts + // command exit codes inside a successful execution response and reserves + // success:false for infrastructure faults, and a command may legitimately exit + // 124 itself. The `--verbose` diagnostic on stderr is what tells a reader a + // timeout fired. + return result; + } +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** + * Derives the container name from a sandbox id. Validates the shape rather than + * trusting it: the id reaches us from persisted session state, and it is about to + * become a `docker` argument. + */ +function containerNameFor(sandboxId: string): string { + const parent = posix.dirname(sandboxId); + const slug = posix.basename(sandboxId); + if (parent !== SANDBOX_PARENT || !SANDBOX_SLUG_RE.test(slug)) { + throw new SandboxNotAvailableError(`not a docker sandbox id: ${sandboxId}`); + } + return `${CONTAINER_NAME_PREFIX}${slug}`; +} + +/** + * Wraps a command so it is confined to `root` and, on timeout, actually dies. + * + * Two problems are solved in one shell invocation: + * + * 1. Symlink escape. Lexical containment on the host cannot see that + * `/link` is a symlink to `/etc/shadow`; `realpath` resolves it and the + * prefix is rechecked against the resolved target. Doing the check and the + * operation in one invocation also removes the check/use window that two + * separate `docker exec` calls would leave open. + * 2. Runaway workloads. Killing the local `docker exec` client does not kill the + * process inside the container (runc#3359), so the timeout is applied by + * `timeout` *inside* the container as well. + */ +function containedCommand(params: { root: string; path: string; command: string; timeoutSeconds: number }): string { + return [ + `__tfy_root=${shellEscape(params.root)}`, + // `-m` allows a not-yet-existing final component (uploads) while still + // resolving symlinks in every component that does exist. + `__tfy_target=$(realpath -m -- ${shellEscape(params.path)})`, + // Quoted variable in the pattern compares literally, not as a glob. + `case "$__tfy_target" in "$__tfy_root"/*) ;; *) echo "path escapes sandbox root" >&2; exit 77 ;; esac`, + // Refuse a symlink even when it resolves inside the root: following one is + // never required here, and allowing it re-opens the swap-after-check window. + `if [ -L "$__tfy_target" ]; then echo "path is a symlink" >&2; exit 77; fi`, + 'export __tfy_target', + `timeout --verbose ${String(params.timeoutSeconds)}s sh -c ${shellEscape(params.command)}`, + ].join('; '); +} + +/** Exit status used by {@link containedCommand} when a path leaves the sandbox. */ +const CONTAINMENT_VIOLATION_EXIT = 77; + +/** + * Spawn a process, collecting stdout as bytes. stdout stays a Buffer because + * downloads carry arbitrary binary content; stderr is decoded because it is only + * ever shown to a human or a model. + */ +async function runProcess(params: { + file: string; + args: readonly string[]; + timeoutMs: number; + stdin?: Buffer; + maxStdoutBytes?: number; +}): Promise { + return new Promise((resolvePromise, rejectPromise) => { + const child = spawn(params.file, [...params.args], { stdio: ['pipe', 'pipe', 'pipe'] }); + + const stdoutChunks: Buffer[] = []; + let stdoutBytes = 0; + let stderr = ''; + let timedOut = false; + let outputCapExceeded = false; + let settled = false; + + const timer = setTimeout(() => { + timedOut = true; + child.kill('SIGKILL'); + }, params.timeoutMs); + + child.stdout.on('data', (chunk: Buffer) => { + stdoutBytes += chunk.length; + if (params.maxStdoutBytes !== undefined && stdoutBytes > params.maxStdoutBytes) { + outputCapExceeded = true; + child.kill('SIGKILL'); + return; + } + stdoutChunks.push(chunk); + }); + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString('utf8'); + }); + + const settle = (fn: () => void): void => { + if (settled) { + return; + } + settled = true; + clearTimeout(timer); + fn(); + }; + + child.on('error', error => { + settle(() => { + rejectPromise(error); + }); + }); + + child.on('close', code => { + settle(() => { + resolvePromise({ + exitCode: code ?? -1, + stdout: Buffer.concat(stdoutChunks), + stderr, + timedOut, + outputCapExceeded, + }); + }); + }); + + if (params.stdin !== undefined) { + child.stdin.end(params.stdin); + } else { + child.stdin.end(); + } + }); +} diff --git a/packages/trueforge/src/sandbox/providerUtils.ts b/packages/trueforge/src/sandbox/providerUtils.ts index 9189f7742..7b6bdab0e 100644 --- a/packages/trueforge/src/sandbox/providerUtils.ts +++ b/packages/trueforge/src/sandbox/providerUtils.ts @@ -1,15 +1,24 @@ -/** Daytona provider construction + persisted build-status refresh (see checkSnapshotStatus). */ +/** Sandbox provider construction + persisted build-status refresh (see checkSnapshotStatus). */ import { Daytona, DaytonaError } from '@daytona/sdk'; -import { DaytonaSandboxProvider, SANDBOX_IMAGE_URI, type SandboxBuild } from '@truefoundry/trueforge-core/core'; +import { + DaytonaSandboxProvider, + SANDBOX_IMAGE_URI, + type SandboxBuild, + type SandboxProvider, +} from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; import configuration from '../config'; import type { ISandboxProviderStore, SandboxProviderRecord } from '../db/sandboxProviderStore'; import { toDaytonaSandboxProviderInput, + toDockerSandboxProviderInput, + type DaytonaSandboxProvider as DaytonaSandboxProviderManifest, + type DockerSandboxProvider as DockerSandboxProviderManifest, type SandboxBuildMetadata, type SandboxProviderManifest, type SandboxStatus, } from '../schemas/sandboxProvider'; +import { DockerSandboxProvider } from './docker/provider/DockerSandboxProvider'; /** Daytona rejected the credentials (401 unauthorized / 403 forbidden); retrying the same key cannot succeed. */ export function isDaytonaAuthError(error: unknown): boolean { @@ -30,7 +39,7 @@ export function toDaytonaSandboxProvider({ logger, build_metadata, }: { - manifest: SandboxProviderManifest; + manifest: DaytonaSandboxProviderManifest; tenant_id: string; logger: Logger; build_metadata?: SandboxBuildMetadata | null; @@ -48,6 +57,52 @@ export function toDaytonaSandboxProvider({ }); } +function toDockerSandboxProvider({ + manifest, + logger, +}: { + manifest: DockerSandboxProviderManifest; + logger: Logger; +}): DockerSandboxProvider { + return new DockerSandboxProvider({ + ...toDockerSandboxProviderInput(manifest), + fileMaxBytesForDownload: configuration.SANDBOX_FILE_MAX_BYTES_FOR_DOWNLOAD, + logger, + }); +} + +/** + * Builds the runtime provider for a stored manifest, whichever backend it names. + * No network or socket I/O until a method is called. + * + * `build_metadata` is Daytona-specific (it pins a built snapshot); the container + * backend has no equivalent because the image tag in the manifest already + * identifies exactly what runs. + */ +export function toSandboxProvider({ + manifest, + tenant_id, + logger, + build_metadata, +}: { + manifest: SandboxProviderManifest; + tenant_id: string; + logger: Logger; + build_metadata?: SandboxBuildMetadata | null; +}): SandboxProvider { + switch (manifest.type) { + case 'daytona': + return toDaytonaSandboxProvider({ + manifest, + tenant_id, + logger, + ...(build_metadata === undefined ? {} : { build_metadata }), + }); + case 'docker': + return toDockerSandboxProvider({ manifest, logger }); + } +} + /** Maps a core `SandboxBuild` onto the persisted/wire status shape (metadata passes through). */ export function toSandboxStatus(build: SandboxBuild): SandboxStatus { return { @@ -90,7 +145,7 @@ export async function checkSnapshotStatus({ return persisted; } - const provider = toDaytonaSandboxProvider({ + const provider = toSandboxProvider({ manifest: record.manifest, tenant_id, logger, diff --git a/packages/trueforge/src/schemas/sandboxCatalog.ts b/packages/trueforge/src/schemas/sandboxCatalog.ts index d4e499350..591c9b8f2 100644 --- a/packages/trueforge/src/schemas/sandboxCatalog.ts +++ b/packages/trueforge/src/schemas/sandboxCatalog.ts @@ -3,14 +3,14 @@ * configured provider manifests in sandboxProvider.ts. */ import { z } from '@hono/zod-openapi'; -import { DaytonaSandboxProviderSchema } from './sandboxProvider'; +import { DaytonaSandboxProviderSchema, DockerSandboxProviderSchema } from './sandboxProvider'; /** - * Catalog wire type. Single variant today (avoids one-member `oneOf` in OpenAPI). - * Widen to a discriminated union when a second provider ships. + * Catalog wire type: presets for discovery, with credentials stripped. Docker has + * no auth field to strip, so it enters the union unchanged. */ -export const CatalogSandboxProviderSchema = DaytonaSandboxProviderSchema.omit({ auth: true }) - .strict() +export const CatalogSandboxProviderSchema = z + .discriminatedUnion('type', [DaytonaSandboxProviderSchema.omit({ auth: true }).strict(), DockerSandboxProviderSchema]) .openapi('CatalogSandboxProvider'); export const SandboxCatalogFileSchema = z diff --git a/packages/trueforge/src/schemas/sandboxProvider.ts b/packages/trueforge/src/schemas/sandboxProvider.ts index a856c9bdc..750777d76 100644 --- a/packages/trueforge/src/schemas/sandboxProvider.ts +++ b/packages/trueforge/src/schemas/sandboxProvider.ts @@ -22,9 +22,8 @@ const DaytonaSandboxProviderAuthSchema = z /** * Daytona-backed sandbox provider config. Persisted as `sandbox_provider.manifest`. - * Left unnamed for OpenAPI so `SandboxProviderManifest` (its single-variant alias) - * is the one emitted component and the response `manifest` field is a plain `$ref` - * instead of an `allOf` wrapper. + * Named for OpenAPI because `SandboxProviderManifest` is now a discriminated union + * and each variant needs its own emitted component. */ export const DaytonaSandboxProviderSchema = z .object({ @@ -47,14 +46,45 @@ export const DaytonaSandboxProviderSchema = z .nonnegative() .describe('Minutes before Daytona auto-deletes the sandbox (0 disables).'), }) - .strict(); + .strict() + .openapi('DaytonaSandboxProvider'); + +/** + * Container-backed sandbox provider config. Unlike Daytona there are no + * credentials: the runtime is a local socket, so authorization is whatever the + * host grants the server process. + * + * A GPU is opt-in. Attaching one to a sandbox that does not need it wastes a + * scarce device and slows container start, so the default is no GPU. + */ +export const DockerSandboxProviderSchema = z + .object({ + type: z.literal('docker').describe('Container-backed sandbox provider (Docker or Podman).'), + image: z.string().min(1).describe('Image the sandbox runs. Must provide a POSIX shell and python3.'), + exec_timeout_ms: z.number().int().positive().describe('Default sandbox command exec timeout in milliseconds.'), + docker_binary: z + .string() + .min(1) + .optional() + .describe('Container CLI to invoke. Defaults to `docker`; set to `podman` or an absolute path.'), + gpus: z.string().min(1).optional().describe('Value passed to `--gpus`, e.g. `all` or `device=0`. Omit for no GPU.'), + extra_run_args: z + .array(z.string()) + .optional() + .describe( + 'Additional `docker run` arguments, e.g. a read-only host mount of a CUDA toolkit so the image can stay small. Never passed through a shell.', + ), + }) + .strict() + .openapi('DockerSandboxProvider'); /** - * Persisted jsonb: the provider config only (no build status). Single variant today — - * this alias carries the OpenAPI name so the spec emits one `SandboxProviderManifest` component. - * Widen to `z.discriminatedUnion('type', [...])` when a second provider ships. + * Persisted jsonb: the provider config only (no build status). Discriminated on + * `type`; every variant is emitted as its own OpenAPI component. */ -export const SandboxProviderManifestSchema = DaytonaSandboxProviderSchema.openapi('SandboxProviderManifest'); +export const SandboxProviderManifestSchema = z + .discriminatedUnion('type', [DaytonaSandboxProviderSchema, DockerSandboxProviderSchema]) + .openapi('SandboxProviderManifest'); /** Named enum so the generated SDK exposes a reusable `SandboxBuildStatus` type. */ export const SandboxBuildStatusSchema = z @@ -104,14 +134,19 @@ export const GetSandboxProviderResponseSchema = z /** Persisted jsonb — the provider config only (no build status). */ export type SandboxProviderManifest = z.infer; export type DaytonaSandboxProvider = z.infer; +export type DockerSandboxProvider = z.infer; export type SandboxBuildStatus = z.infer; export type SandboxBuildMetadata = z.infer; export type SandboxStatus = z.infer; export type ConfiguredSandboxProvider = z.infer; export type UpdateSandboxProviderRequest = z.infer; -/** Wire/persisted snake_case → Daytona client credentials + provider settings. */ -export function toDaytonaSandboxProviderInput(manifest: SandboxProviderManifest): { +/** + * Wire/persisted snake_case → Daytona client credentials + provider settings. + * Takes the narrowed variant, not the union: the caller has already discriminated + * on `type`, and accepting the union here would push an unchecked cast inward. + */ +export function toDaytonaSandboxProviderInput(manifest: DaytonaSandboxProvider): { apiKey: string; } & Pick< DaytonaSandboxProviderOptions, @@ -125,3 +160,22 @@ export function toDaytonaSandboxProviderInput(manifest: SandboxProviderManifest) autoDeleteIntervalInMinutes: manifest.auto_delete_interval_in_minutes, }; } + +/** Wire/persisted snake_case → container provider constructor options. */ +export function toDockerSandboxProviderInput(manifest: DockerSandboxProvider): { + image: string; + execTimeoutSeconds: number; + dockerBinary?: string; + gpus?: string; + extraRunArgs?: readonly string[]; +} { + return { + image: manifest.image, + // The wire field is milliseconds for symmetry with Daytona; the provider + // takes seconds because that is what `docker exec` timeouts are reasoned in. + execTimeoutSeconds: Math.max(1, Math.round(manifest.exec_timeout_ms / 1000)), + ...(manifest.docker_binary === undefined ? {} : { dockerBinary: manifest.docker_binary }), + ...(manifest.gpus === undefined ? {} : { gpus: manifest.gpus }), + ...(manifest.extra_run_args === undefined ? {} : { extraRunArgs: manifest.extra_run_args }), + }; +} diff --git a/packages/trueforge/tests/db/sandboxProviderStoreContractSuite.ts b/packages/trueforge/tests/db/sandboxProviderStoreContractSuite.ts index a337d1c0c..d632f76e1 100644 --- a/packages/trueforge/tests/db/sandboxProviderStoreContractSuite.ts +++ b/packages/trueforge/tests/db/sandboxProviderStoreContractSuite.ts @@ -3,7 +3,11 @@ * Runs under jest against a fresh store per test (see backend test files). */ import type { ISandboxProviderStore, UpsertSandboxProviderInput } from '../../src/db/sandboxProviderStore'; -import type { SandboxBuildMetadata, SandboxProviderManifest } from '../../src/schemas/sandboxProvider'; +import type { + DaytonaSandboxProvider, + SandboxBuildMetadata, + SandboxProviderManifest, +} from '../../src/schemas/sandboxProvider'; const TENANT = 'default'; @@ -12,7 +16,7 @@ const BUILD_METADATA: SandboxBuildMetadata = { image_uri: 'tfy.jfrog.io/tfy-images/sandbox:029ea5ff', }; -function manifest(overrides: Partial = {}): SandboxProviderManifest { +function manifest(overrides: Partial = {}): SandboxProviderManifest { return { type: 'daytona', auth: { api_key: 'dtn-test' }, diff --git a/packages/trueforge/tests/unit/apis/sandboxProviders.test.ts b/packages/trueforge/tests/unit/apis/sandboxProviders.test.ts index fa2220862..633557e16 100644 --- a/packages/trueforge/tests/unit/apis/sandboxProviders.test.ts +++ b/packages/trueforge/tests/unit/apis/sandboxProviders.test.ts @@ -1,9 +1,9 @@ // Stub the Daytona-touching helpers so the router never talks to Daytona: the PUT path builds via -// toDaytonaSandboxProvider, and the GET path refreshes via checkSnapshotStatus. isDaytonaAuthError +// toSandboxProvider, and the GET path refreshes via checkSnapshotStatus. isDaytonaAuthError // and toSandboxStatus stay real so the auth-error mapping and PUT wire shape are exercised. jest.mock('../../../src/sandbox/providerUtils', () => { const actual = jest.requireActual('../../../src/sandbox/providerUtils'); - return { ...actual, toDaytonaSandboxProvider: jest.fn(), checkSnapshotStatus: jest.fn() }; + return { ...actual, toSandboxProvider: jest.fn(), checkSnapshotStatus: jest.fn() }; }); import { DaytonaError } from '@daytona/sdk'; @@ -20,10 +20,10 @@ import { migrateSqliteToLatest } from '../../../src/db/migrateSqlite'; import type { ISandboxProviderStore } from '../../../src/db/sandboxProviderStore'; import { createSqliteDb } from '../../../src/db/sqlite/client'; import { SqliteSandboxProviderStore } from '../../../src/db/sqlite/sandbox-provider-store/SqliteSandboxProviderStore'; -import { checkSnapshotStatus, toDaytonaSandboxProvider } from '../../../src/sandbox/providerUtils'; +import { checkSnapshotStatus, toSandboxProvider } from '../../../src/sandbox/providerUtils'; import { toRedactedSecretValue } from '../../../src/utils/secretRedaction'; -const mockProviderFactory = toDaytonaSandboxProvider as jest.Mock; +const mockProviderFactory = toSandboxProvider as jest.Mock; const mockCheckStatus = checkSnapshotStatus as jest.Mock; const silentLogger = createLogger({ silent: true }); @@ -249,7 +249,11 @@ describe('sandbox-provider secret redaction and strict PUT', () => { }); const stored = await sandboxProviderStore.getSandboxProvider(TENANT_ID); - expect(stored?.manifest.auth.api_key).toBe(rotatedKey); + // The manifest is a discriminated union now and only daytona carries auth. + if (stored?.manifest.type !== 'daytona') { + throw new Error('expected a stored daytona manifest'); + } + expect(stored.manifest.auth.api_key).toBe(rotatedKey); }); it('PUT update reuses persisted build_metadata (no image upgrade on re-save)', async () => { diff --git a/packages/trueforge/tests/unit/sandbox/docker/provider/gpu.contract.test.ts b/packages/trueforge/tests/unit/sandbox/docker/provider/gpu.contract.test.ts new file mode 100644 index 000000000..e021a9939 --- /dev/null +++ b/packages/trueforge/tests/unit/sandbox/docker/provider/gpu.contract.test.ts @@ -0,0 +1,175 @@ +import { createLogger } from 'winston'; +import { DockerSandboxProvider } from '../../../../../src/sandbox/docker/provider/DockerSandboxProvider'; + +/** + * Proves `gpus` actually reaches the device, by compiling and running a kernel and + * checking the bandwidth it reports, rather than trusting that `--gpus` was passed. + * + * Skipped unless a GPU host is present, so it is safe in CI. + */ + +const IMAGE = process.env['TFY_DOCKER_SANDBOX_GPU_IMAGE'] ?? 'nvidia/cuda:13.0.0-devel-ubuntu24.04'; +const GPUS = process.env['TFY_DOCKER_SANDBOX_GPUS'] ?? 'all'; + +/** float4 device-to-device copy: the simplest kernel that saturates memory. */ +const BANDWIDTH_KERNEL = String.raw` +#include +#include +__global__ void copyk(const float4* __restrict__ in, float4* __restrict__ out, size_t n4) { + size_t i = blockIdx.x * (size_t)blockDim.x + threadIdx.x; + if (i < n4) out[i] = in[i]; +} +int main() { + size_t bytes = 256ull << 20; + size_t n4 = bytes / sizeof(float4); + float4 *d_in, *d_out; + if (cudaMalloc(&d_in, bytes) != cudaSuccess) { printf("ALLOC_FAIL\n"); return 1; } + if (cudaMalloc(&d_out, bytes) != cudaSuccess) { printf("ALLOC_FAIL\n"); return 1; } + cudaMemset(d_in, 1, bytes); + int block = 256; + size_t grid = (n4 + block - 1) / block; + for (int i = 0; i < 3; i++) copyk<<>>(d_in, d_out, n4); + cudaDeviceSynchronize(); + cudaEvent_t a, b; + cudaEventCreate(&a); cudaEventCreate(&b); + int iters = 30; + cudaEventRecord(a); + for (int i = 0; i < iters; i++) copyk<<>>(d_in, d_out, n4); + cudaEventRecord(b); + cudaEventSynchronize(b); + float ms = 0; + cudaEventElapsedTime(&ms, a, b); + double gbps = (2.0 * bytes) / (ms / 1000.0 / iters) / 1e9; + int mclk = 0, bus = 0; + cudaDeviceGetAttribute(&mclk, cudaDevAttrMemoryClockRate, 0); + cudaDeviceGetAttribute(&bus, cudaDevAttrGlobalMemoryBusWidth, 0); + double peak = 2.0 * (double)mclk * 1e3 * (bus / 8.0) / 1e9; + printf("ACHIEVED_GBPS=%.1f\nPEAK_GBPS=%.1f\nPCT_SOL=%.1f\nSTATUS=%s\n", + gbps, peak, 100.0 * gbps / peak, cudaGetErrorString(cudaGetLastError())); + return 0; +} +`; + +describe('DockerSandboxProvider GPU', () => { + let provider: DockerSandboxProvider | undefined; + let sandboxId: string | undefined; + let available = false; + + beforeAll(async () => { + const support = await DockerSandboxProvider.isSupported(); + if (!support.supported) { + return; + } + const candidate = new DockerSandboxProvider({ + image: IMAGE, + gpus: GPUS, + logger: createLogger({ silent: true }), + execTimeoutSeconds: 300, + }); + const build = await candidate.getImageBuildStatus(); + if (build.status !== 'ready') { + return; + } + try { + const created = await candidate.createSandbox(); + sandboxId = created.sandboxId; + provider = candidate; + available = true; + } catch { + await candidate.dispose(); + } + }, 600_000); + + afterAll(async () => { + await provider?.dispose(); + }, 120_000); + + it('exposes the GPU to nvidia-smi inside the sandbox', async () => { + if (!available || !provider || !sandboxId) { + pending('no GPU-capable docker host'); + return; + } + const result = await provider.exec({ + sandboxId, + command: 'nvidia-smi --query-gpu=name --format=csv,noheader', + }); + expect(result.success).toBe(true); + if (!result.success) { + throw new Error('unreachable'); + } + expect(result.response.exitCode).toBe(0); + expect(result.response.result).toMatch(/NVIDIA/); + }, 120_000); + + it('compiles and runs a CUDA kernel at plausible bandwidth', async () => { + if (!available || !provider || !sandboxId) { + pending('no GPU-capable docker host'); + return; + } + // Upload rather than heredoc the source: this is the path a real kernel + // project takes, and it exercises uploadFile with non-trivial content. + await provider.uploadFile({ + sandboxId, + remotePath: 'bw.cu', + content: Buffer.from(BANDWIDTH_KERNEL, 'utf8'), + }); + + const arch = process.env['TFY_DOCKER_SANDBOX_GPU_ARCH'] ?? 'sm_89'; + const built = await provider.exec({ + sandboxId, + command: `nvcc -O3 -arch=${arch} bw.cu -o bw`, + timeoutSeconds: 300, + }); + expect(built.success).toBe(true); + if (!built.success) { + throw new Error('unreachable'); + } + expect(built.response.exitCode).toBe(0); + + const ran = await provider.exec({ sandboxId, command: './bw', timeoutSeconds: 300 }); + expect(ran.success).toBe(true); + if (!ran.success) { + throw new Error('unreachable'); + } + expect(ran.response.exitCode).toBe(0); + + const out = ran.response.result; + // Printed on purpose. This project's thesis is that a performance claim you + // cannot see is a performance claim you cannot trust; that applies to its own + // test suite too. + // eslint-disable-next-line no-console + console.log(`[gpu.smoke] nvcc -arch=${arch}\n${out.trim()}`); + expect(out).toContain('STATUS=no error'); + + const pct = Number(/PCT_SOL=([\d.]+)/.exec(out)?.[1]); + expect(Number.isFinite(pct)).toBe(true); + // Above 100% of theoretical peak means the measurement is wrong, which is + // the whole premise of the roofline gate. Below 40% means the GPU is being + // emulated or throttled hard enough that the sandbox is not usable for + // benchmarking. + expect(pct).toBeGreaterThan(40); + expect(pct).toBeLessThanOrEqual(100); + }, 600_000); + + it('round-trips a payload larger than the argv limit', async () => { + if (!available || !provider || !sandboxId) { + pending('no GPU-capable docker host'); + return; + } + // Regression guard for the failure mode in upstream issue #416: providers + // that base64 the payload into a single argv die past roughly 96 KiB on + // MAX_ARG_STRLEN / E2BIG. 5 MiB of non-repeating bytes is well past that and + // also defeats any accidental compression. + const size = 5 * 1024 * 1024; + const payload = Buffer.alloc(size); + for (let i = 0; i < size; i++) { + payload[i] = (i * 31 + (i >> 8)) & 0xff; + } + + await provider.uploadFile({ sandboxId, remotePath: 'big.bin', content: payload }); + const downloaded = await provider.downloadFile({ sandboxId, path: 'big.bin' }); + + expect(downloaded.length).toBe(size); + expect(Buffer.compare(downloaded, payload)).toBe(0); + }, 300_000); +}); diff --git a/packages/trueforge/tests/unit/sandbox/docker/provider/hardening.contract.test.ts b/packages/trueforge/tests/unit/sandbox/docker/provider/hardening.contract.test.ts new file mode 100644 index 000000000..0ca23e7ba --- /dev/null +++ b/packages/trueforge/tests/unit/sandbox/docker/provider/hardening.contract.test.ts @@ -0,0 +1,277 @@ +import { createLogger } from 'winston'; +import { DockerSandboxProvider } from '../../../../../src/sandbox/docker/provider/DockerSandboxProvider'; + +/** + * Regression guards for issues found in code review of the initial provider. + * Each test corresponds to a specific defect, named in its description. + */ + +const IMAGE = process.env['TFY_DOCKER_SANDBOX_TEST_IMAGE'] ?? 'nvidia/cuda:13.0.0-base-ubuntu24.04'; + +/** + * Every provider in this suite gets a scope unique to the run. The reaper only + * removes containers in its own scope, so this suite cannot delete sandboxes + * belonging to another test worker or to a dev server on the same daemon. + */ +const TEST_SCOPE = `test-${String(process.pid)}-${String(Date.now())}`; + +function newProvider(overrides: { execTimeoutSeconds?: number } = {}): DockerSandboxProvider { + return new DockerSandboxProvider({ + image: IMAGE, + scope: TEST_SCOPE, + logger: createLogger({ silent: true }), + ...overrides, + }); +} + +describe('DockerSandboxProvider hardening', () => { + let supported = false; + + beforeAll(async () => { + supported = (await DockerSandboxProvider.isSupported()).supported; + }, 60_000); + + it('addresses a sandbox from a provider instance that did not create it', async () => { + if (!supported) { + pending('no docker host'); + return; + } + // The server builds a fresh provider per turn and hands it a sandbox id + // carried over from an earlier turn. An instance-local id->container map + // would be empty exactly when it was needed. + const creator = newProvider(); + const laterTurn = newProvider(); + try { + const { sandboxId } = await creator.createSandbox(); + + const write = await creator.exec({ sandboxId, command: "printf 'turn-one\\n' > carried.txt" }); + expect(write.success).toBe(true); + + const read = await laterTurn.exec({ sandboxId, command: 'cat carried.txt' }); + expect(read.success).toBe(true); + if (!read.success) { + throw new Error('unreachable'); + } + expect(read.response.exitCode).toBe(0); + expect(read.response.result).toContain('turn-one'); + } finally { + await creator.dispose(); + await laterTurn.dispose(); + } + }, 180_000); + + it('rejects a sandbox id it does not own instead of shelling out with it', async () => { + if (!supported) { + pending('no docker host'); + return; + } + const provider = newProvider(); + for (const bogus of ['/etc', '/sandbox/../etc', '/sandbox/not-a-ulid', '/sandbox/x; rm -rf /']) { + // SandboxNotAvailableError propagates rather than being folded into a + // success:false result, matching the local provider's behaviour. + await expect(provider.exec({ sandboxId: bogus, command: 'echo reached' })).rejects.toThrow( + /not a docker sandbox id/, + ); + } + }, 120_000); + + it('refuses to read through a symlink that escapes the sandbox root', async () => { + if (!supported) { + pending('no docker host'); + return; + } + const provider = newProvider(); + try { + const { sandboxId } = await provider.createSandbox(); + + // Lexically this path is inside the sandbox; only resolution reveals it is not. + const link = await provider.exec({ sandboxId, command: 'ln -s /etc/passwd escape.txt' }); + expect(link.success).toBe(true); + + await expect(provider.downloadFile({ sandboxId, path: 'escape.txt' })).rejects.toThrow(); + + // Same for a symlinked directory component. + const dirLink = await provider.exec({ sandboxId, command: 'ln -s /etc etcdir' }); + expect(dirLink.success).toBe(true); + await expect(provider.downloadFile({ sandboxId, path: 'etcdir/passwd' })).rejects.toThrow(); + } finally { + await provider.dispose(); + } + }, 180_000); + + it('refuses to write through a symlink that escapes the sandbox root', async () => { + if (!supported) { + pending('no docker host'); + return; + } + const provider = newProvider(); + try { + const { sandboxId } = await provider.createSandbox(); + const link = await provider.exec({ sandboxId, command: 'ln -s /tmp/pwned outfile' }); + expect(link.success).toBe(true); + + await expect( + provider.uploadFile({ sandboxId, remotePath: 'outfile', content: Buffer.from('nope') }), + ).rejects.toThrow(); + + const leaked = await provider.exec({ sandboxId, command: 'test -e /tmp/pwned' }); + expect(leaked.success).toBe(true); + if (!leaked.success) { + throw new Error('unreachable'); + } + expect(leaked.response.exitCode).not.toBe(0); + } finally { + await provider.dispose(); + } + }, 180_000); + + it('kills the timed-out workload inside the container, not just the client', async () => { + if (!supported) { + pending('no docker host'); + return; + } + // Killing `docker exec` locally leaves the command running inside the + // container (runc#3359), so the bound is applied in-container as well. + // + // Scope: this covers the foreground workload, which is what a compile or a + // benchmark is. A command that deliberately detaches a child (`cmd &` then + // exits) still outlives the timeout, because `timeout` signals its own child + // rather than the process group; those are bounded by container removal and + // by reapStale rather than by this mechanism. + const provider = newProvider({ execTimeoutSeconds: 2 }); + try { + const { sandboxId } = await provider.createSandbox(); + + const started = Date.now(); + const result = await provider.exec({ sandboxId, command: 'sleep 120' }); + const elapsed = Date.now() - started; + + // Returned on the in-container bound, nowhere near the 120s the command asked for. + expect(elapsed).toBeLessThan(30_000); + // A timeout is a command outcome, not an infrastructure fault, so it rides in + // a successful envelope carrying exit 124 plus `timeout --verbose`'s note. + expect(result.success).toBe(true); + if (!result.success) { + throw new Error('unreachable'); + } + expect(result.response.exitCode).toBe(124); + expect(result.response.result).toMatch(/timeout: sending signal/); + + // The workload itself is gone, not merely detached from its client. + // The bracket idiom keeps the pattern from matching this command's own + // argv, which `ps` would otherwise list and grep would happily count. + const survivors = await provider.exec({ + sandboxId, + command: 'ps -eo args= 2>/dev/null | grep -c "[s]leep 120" || true', + timeoutSeconds: 30, + }); + expect(survivors.success).toBe(true); + if (!survivors.success) { + throw new Error('unreachable'); + } + expect(survivors.response.result.trim()).toBe('0'); + } finally { + await provider.dispose(); + } + }, 180_000); + + it('refuses a cutoff that would mark live sandboxes stale', async () => { + await expect( + DockerSandboxProvider.reapStale({ + scope: TEST_SCOPE, + olderThanMs: -1, + logger: createLogger({ silent: true }), + }), + ).rejects.toThrow(RangeError); + }); + + it('reaps only sandboxes in its own scope', async () => { + if (!supported) { + pending('no docker host'); + return; + } + const mine = newProvider(); + // Stands in for another server or test worker sharing the daemon. Its sandbox + // must survive a reap aimed at TEST_SCOPE. + const other = new DockerSandboxProvider({ + image: IMAGE, + scope: `${TEST_SCOPE}-bystander`, + logger: createLogger({ silent: true }), + }); + try { + const { sandboxId } = await mine.createSandbox(); + const bystander = await other.createSandbox(); + + const { removed } = await DockerSandboxProvider.reapStale({ + scope: TEST_SCOPE, + olderThanMs: 0, + logger: createLogger({ silent: true }), + }); + expect(removed.length).toBeGreaterThan(0); + + // Mine is gone... + const afterReap = await mine.exec({ sandboxId, command: 'echo alive' }); + if (afterReap.success) { + expect(afterReap.response.exitCode).not.toBe(0); + } + + // ...and the bystander's is untouched. + const survived = await other.exec({ sandboxId: bystander.sandboxId, command: 'echo alive' }); + expect(survived.success).toBe(true); + if (!survived.success) { + throw new Error('unreachable'); + } + expect(survived.response.exitCode).toBe(0); + } finally { + await mine.dispose(); + await other.dispose(); + } + }, 240_000); + + it('reports a command that exits 124 as itself, not as a timeout', async () => { + if (!supported) { + pending('no docker host'); + return; + } + // GNU timeout exits 124 when it fires and also passes a command's own 124 + // through, so status alone cannot tell them apart. + const provider = newProvider(); + try { + const { sandboxId } = await provider.createSandbox(); + const result = await provider.exec({ sandboxId, command: 'exit 124' }); + expect(result.success).toBe(true); + if (!result.success) { + throw new Error('unreachable'); + } + expect(result.response.exitCode).toBe(124); + // No timeout diagnostic, because nothing timed out. + expect(result.response.result).not.toMatch(/timeout: sending signal/); + } finally { + await provider.dispose(); + } + }, 180_000); + + it('reports an oversized download as too large, not missing', async () => { + if (!supported) { + pending('no docker host'); + return; + } + // Classification must survive both enforcement layers: the in-shell size + // check, and the streaming cap that catches a file growing mid-`cat`. + const provider = new DockerSandboxProvider({ + image: IMAGE, + scope: TEST_SCOPE, + logger: createLogger({ silent: true }), + fileMaxBytesForDownload: 1024, + }); + try { + const { sandboxId } = await provider.createSandbox(); + const made = await provider.exec({ sandboxId, command: 'head -c 65536 /dev/zero > big.bin' }); + expect(made.success).toBe(true); + + await expect(provider.downloadFile({ sandboxId, path: 'big.bin' })).rejects.toThrow(/too large|exceeds/i); + } finally { + await provider.dispose(); + } + }, 180_000); +}); diff --git a/packages/trueforge/tests/unit/sandbox/docker/provider/provider.contract.test.ts b/packages/trueforge/tests/unit/sandbox/docker/provider/provider.contract.test.ts new file mode 100644 index 000000000..93f439a45 --- /dev/null +++ b/packages/trueforge/tests/unit/sandbox/docker/provider/provider.contract.test.ts @@ -0,0 +1,35 @@ +import { createLogger } from 'winston'; +import { runSandboxProviderContractSuite } from '../../../../../../trueforge-core/tests/core/sandbox/provider/sandboxProviderContractSuite'; +import { DockerSandboxProvider } from '../../../../../src/sandbox/docker/provider/DockerSandboxProvider'; + +/** + * Image is overridable so CI can pin something small. The default only needs a + * POSIX shell and coreutils -- the contract suite never invokes Python, and + * requiring a CUDA image here would make the suite depend on a GPU host. + */ +const IMAGE = process.env['TFY_DOCKER_SANDBOX_TEST_IMAGE'] ?? 'nvidia/cuda:13.0.0-base-ubuntu24.04'; + +describe('DockerSandboxProvider (SandboxProvider contract)', () => { + runSandboxProviderContractSuite(async () => { + const support = await DockerSandboxProvider.isSupported(); + if (!support.supported) { + pending(`Docker sandbox not supported: ${support.reason}`); + throw new Error(support.reason); + } + const provider = new DockerSandboxProvider({ + image: IMAGE, + logger: createLogger({ silent: true }), + }); + const build = await provider.buildImage(); + if (build.status === 'failed') { + pending(`image unavailable: ${build.reason ?? 'unknown'}`); + throw new Error(build.reason ?? 'image build failed'); + } + return { + provider, + dispose: async () => { + await provider.dispose(); + }, + }; + }); +});