From 0640e7202b4c250a19dcee2e46e203627da5fc60 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 10:43:15 -0400 Subject: [PATCH 1/8] =?UTF-8?q?=F0=9F=8F=B7=EF=B8=8F=20feat:=20Add=20tagge?= =?UTF-8?q?d=20releases=20(#77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deployments currently have to track main, which advances whenever an internal snapshot is merged. Cut versioned tags instead, each carrying the packaged Helm chart so a deployment can pin one. The tag is the app version and must match helm/codeapi/Chart.yaml appVersion, so a deployed chart cannot report a version no release ever carried. `latest` moves only for the highest stable tag, and the chart is packaged before the tag is created so a rate-limited subchart pull leaves the version unused and the run retryable. Closes #63 --- .github/release.yml | 31 +++++ .github/workflows/release.yml | 252 ++++++++++++++++++++++++++++++++++ .gitignore | 1 + CONTRIBUTING.md | 8 ++ README.md | 20 +++ docs/RELEASING.md | 71 ++++++++++ 6 files changed, 383 insertions(+) create mode 100644 .github/release.yml create mode 100644 .github/workflows/release.yml create mode 100644 docs/RELEASING.md diff --git a/.github/release.yml b/.github/release.yml new file mode 100644 index 00000000..0a59193a --- /dev/null +++ b/.github/release.yml @@ -0,0 +1,31 @@ +# Categories for the changelog `gh release create --generate-notes` appends to +# every release body (see .github/workflows/release.yml). Labels are matched +# against the merged pull requests in the range; anything unlabelled lands in +# "Other changes" rather than being dropped, which matters here because sync +# pull requests from the internal monorepo usually carry no labels. +changelog: + exclude: + labels: + - duplicate + - invalid + - wontfix + categories: + - title: Security + labels: + - security + - title: Features + labels: + - enhancement + - feature + - title: Fixes + labels: + - bug + - title: Documentation + labels: + - documentation + - title: Dependencies + labels: + - dependencies + - title: Other changes + labels: + - '*' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..55c7a116 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,252 @@ +# Cuts tagged releases for the public code-interpreter repo. Like ci.yml this +# file is inert inside the monorepo — GitHub only runs workflows from the repo +# root — and becomes a root workflow in the published repo. +# +# Two entry points feed one job: +# +# * workflow_dispatch — pick a version in the Actions UI. The chart is +# packaged before the tag is created, so a packaging failure aborts while +# the release is still un-cut and the version is still free to reuse. +# * push of a v* tag — for tags cut locally with `git tag -a … && git push`. +# Tags this workflow pushes itself carry GITHUB_TOKEN, and GitHub does not +# re-trigger workflows for those, so the two paths never double-publish. +# +# `main` accepts no direct pushes (see CONTRIBUTING.md), but the branch +# ruleset does not cover tags, so the job can create them. GITHUB_TOKEN +# defaults to read-only in this repository; the explicit `contents: write` +# below is what lets the tag push and the release upload through. +name: Release + +on: + workflow_dispatch: + inputs: + version: + description: 'Version to release, e.g. v2.0.0 or v2.1.0-rc1. Must match helm/codeapi/Chart.yaml appVersion.' + required: true + type: string + draft: + description: 'Publish as a draft so the notes can be edited before going public' + type: boolean + default: false + push: + tags: + - 'v*' + +permissions: + contents: write + +concurrency: + group: release-${{ github.event.inputs.version || github.ref_name }} + cancel-in-progress: false + +jobs: + release: + name: Tag and publish + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + # Full history and tags: resolving whether this release is the newest + # stable one compares it against every other tag in the repository. + fetch-depth: 0 + + - name: Resolve and validate version + id: version + env: + EVENT_NAME: ${{ github.event_name }} + INPUT_VERSION: ${{ github.event.inputs.version }} + INPUT_DRAFT: ${{ github.event.inputs.draft }} + REF_NAME: ${{ github.ref_name }} + REF_TYPE: ${{ github.ref_type }} + run: | + set -euo pipefail + + if [ "$EVENT_NAME" = "workflow_dispatch" ]; then + # Releases describe what shipped to main. Dispatching from a topic + # branch would tag a commit that is not on the release line. + if [ "$REF_TYPE" != "branch" ] || [ "$REF_NAME" != "main" ]; then + echo "::error::Releases must be cut from main; this run is on '$REF_NAME'" + exit 1 + fi + VERSION="$INPUT_VERSION" + else + VERSION="$REF_NAME" + fi + + # A bare "2.0.0" typed into the dispatch box is accepted; everything + # downstream works with the v-prefixed form the tag actually uses. + case "$VERSION" in + v*) ;; + *) VERSION="v$VERSION" ;; + esac + + if [[ ! "$VERSION" =~ ^v[0-9]+[.][0-9]+[.][0-9]+(-rc[0-9]+)?$ ]]; then + echo "::error::Release tags must be v.. or v..-rcN, for example v2.0.0 or v2.1.0-rc1 (got '$VERSION')" + exit 1 + fi + + # v2.1.0-rc1 -> 2.1.0. Release candidates carry the version they are + # candidates for, so they compare against the same appVersion. + BASE_VERSION="${VERSION%%-rc*}" + BASE_VERSION="${BASE_VERSION#v}" + + read_chart_field() { + grep -m1 "^$1:" helm/codeapi/Chart.yaml \ + | sed -E "s/^$1:[[:space:]]*//; s/[[:space:]]*#.*//; s/^[\"']//; s/[\"']\$//" + } + APP_VERSION="$(read_chart_field appVersion)" + CHART_VERSION="$(read_chart_field version)" + + # The tag is the app version. Requiring the bump to have landed on + # main first keeps a deployed chart from reporting a version that no + # release ever carried. + if [ "$APP_VERSION" != "$BASE_VERSION" ]; then + echo "::error::Tag $VERSION does not match helm/codeapi/Chart.yaml appVersion ($APP_VERSION). Land the appVersion bump on main before releasing." + exit 1 + fi + + if [ "$EVENT_NAME" = "workflow_dispatch" ] \ + && git rev-parse -q --verify "refs/tags/$VERSION" >/dev/null; then + echo "::error::Tag $VERSION already exists. Pick a new version, or delete the tag if it was cut in error." + exit 1 + fi + + case "$VERSION" in + *-rc*) PRERELEASE=true ;; + *) PRERELEASE=false ;; + esac + + # `latest` moves only when this is the highest stable version, so + # re-cutting an older patch cannot drag it backwards. The tag under + # dispatch does not exist yet, hence adding it to the comparison. + LATEST=false + if [ "$PRERELEASE" = "false" ]; then + HIGHEST_STABLE="$( + { + git tag --list 'v[0-9]*' + printf '%s\n' "$VERSION" + } \ + | grep -E '^v[0-9]+[.][0-9]+[.][0-9]+$' \ + | sort -V \ + | tail -n 1 + )" + if [ "$HIGHEST_STABLE" = "$VERSION" ]; then + LATEST=true + fi + fi + + DRAFT=false + if [ "$INPUT_DRAFT" = "true" ]; then + DRAFT=true + fi + + { + echo "version=$VERSION" + echo "base_version=$BASE_VERSION" + echo "app_version=$APP_VERSION" + echo "chart_version=$CHART_VERSION" + echo "prerelease=$PRERELEASE" + echo "latest=$LATEST" + echo "draft=$DRAFT" + } >> "$GITHUB_OUTPUT" + + echo "Releasing $VERSION (chart $CHART_VERSION, appVersion $APP_VERSION, prerelease=$PRERELEASE, latest=$LATEST, draft=$DRAFT)" + + # helm is preinstalled on ubuntu-latest, the same way the chart tests in + # ci.yml depend on it. + - name: Package Helm chart + id: chart + run: | + set -euo pipefail + + # Subcharts resolve through the Bitnami OCI mirror on Docker Hub, + # which rate-limits anonymous pulls. A transient 429 should cost a + # retry, not the release. + for attempt in 1 2 3; do + if helm dependency update helm/codeapi; then + break + fi + if [ "$attempt" = 3 ]; then + echo "::error::helm dependency update failed after 3 attempts" + exit 1 + fi + sleep $(( attempt * 15 )) + done + + helm package helm/codeapi --destination dist + + CHART_PATH="$(ls dist/codeapi-*.tgz)" + { + echo "path=$CHART_PATH" + echo "name=$(basename "$CHART_PATH")" + } >> "$GITHUB_OUTPUT" + + - name: Create tag + if: github.event_name == 'workflow_dispatch' + env: + VERSION: ${{ steps.version.outputs.version }} + run: | + set -euo pipefail + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + git tag -a "$VERSION" -m "$VERSION" + git push origin "refs/tags/$VERSION" + + - name: Publish release + env: + GH_TOKEN: ${{ github.token }} + VERSION: ${{ steps.version.outputs.version }} + APP_VERSION: ${{ steps.version.outputs.app_version }} + CHART_VERSION: ${{ steps.version.outputs.chart_version }} + CHART_PATH: ${{ steps.chart.outputs.path }} + CHART_NAME: ${{ steps.chart.outputs.name }} + PRERELEASE: ${{ steps.version.outputs.prerelease }} + LATEST: ${{ steps.version.outputs.latest }} + DRAFT: ${{ steps.version.outputs.draft }} + REPO_URL: ${{ github.server_url }}/${{ github.repository }} + run: | + set -euo pipefail + + if gh release view "$VERSION" >/dev/null 2>&1; then + echo "::error::Release $VERSION already exists" + exit 1 + fi + + # Quoted heredoc so the markdown backticks stay literal; the + # placeholders are filled in afterwards. + cat > release-notes.md <<'NOTES' + Pin deployments to this tag instead of tracking `main`: + + ```bash + git clone --branch __VERSION__ --depth 1 __REPO_URL__.git + ``` + + The attached `__CHART_NAME__` is the packaged Helm chart (chart `__CHART_VERSION__`, appVersion `__APP_VERSION__`) with its Redis and MinIO subcharts vendored, so it installs without adding any chart repositories: + + ```bash + helm install codeapi ./__CHART_NAME__ -f my-values.yaml + ``` + + Chart configuration is documented in [helm/codeapi/README.md](__REPO_URL__/blob/__VERSION__/helm/codeapi/README.md). + NOTES + + sed -i \ + -e "s|__VERSION__|$VERSION|g" \ + -e "s|__REPO_URL__|$REPO_URL|g" \ + -e "s|__CHART_NAME__|$CHART_NAME|g" \ + -e "s|__CHART_VERSION__|$CHART_VERSION|g" \ + -e "s|__APP_VERSION__|$APP_VERSION|g" \ + release-notes.md + + # --generate-notes appends the merged-pull-request changelog below + # the body from --notes-file, categorised per .github/release.yml. + gh release create "$VERSION" \ + --title "$VERSION" \ + --notes-file release-notes.md \ + --generate-notes \ + --verify-tag \ + --prerelease="$PRERELEASE" \ + --latest="$LATEST" \ + --draft="$DRAFT" \ + "$CHART_PATH#Helm chart ($CHART_NAME)" diff --git a/.gitignore b/.gitignore index a1a0c6ed..958b3332 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ packages/*/dist/ # Helm artifacts helm/*/charts/*.tgz helm/*/Chart.lock +/dist/ # Local sandbox runtime data (docker volume mount) data/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 441ba55d..45c85202 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,6 +22,14 @@ Practical consequences: - **History is snapshot-based.** Commits here intentionally do not mirror the internal commit history. +## Releases + +Tagged releases are cut from `main` as `vMAJOR.MINOR.PATCH` (with `-rcN` for +release candidates), and each one carries the packaged Helm chart. The version +comes from `helm/codeapi/Chart.yaml`'s `appVersion`, so a version bump lands on +`main` through the pull request flow above before it can be released. See +[docs/RELEASING.md](docs/RELEASING.md) for the full process. + ## Development See the [README](README.md) for the architecture overview and diff --git a/README.md b/README.md index 384e013a..113cbb0e 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,26 @@ privilege, keep hosts patched, and deploy responsibly. If you believe you have found a vulnerability, please report it privately rather than opening a public issue (see [CONTRIBUTING](CONTRIBUTING.md)). +## Releases + +Deployments should pin a [tagged release](https://github.com/LibreChat-AI/code-interpreter/releases) +rather than track `main`, which moves whenever an internal snapshot is merged: + +```bash +git clone --branch v2.0.0 --depth 1 https://github.com/LibreChat-AI/code-interpreter.git +``` + +Every release attaches `codeapi-.tgz`, the packaged Helm chart +with its Redis and MinIO subcharts vendored: + +```bash +helm install codeapi ./codeapi-0.3.0.tgz -f my-values.yaml +``` + +Versions are `vMAJOR.MINOR.PATCH`, with `-rcN` release candidates published as +pre-releases. See [docs/RELEASING.md](docs/RELEASING.md) for how releases are +cut. + ## Local Development ```bash diff --git a/docs/RELEASING.md b/docs/RELEASING.md new file mode 100644 index 00000000..dd00c96d --- /dev/null +++ b/docs/RELEASING.md @@ -0,0 +1,71 @@ +# Releasing + +Deployments should track a tag, not `main`. This document covers how those +tags are cut. + +## Versioning + +A release is named `vMAJOR.MINOR.PATCH`, optionally with a `-rcN` suffix for a +release candidate — `v2.0.0`, `v2.1.0-rc1`. That version is the **app +version**: `helm/codeapi/Chart.yaml`'s `appVersion` is its source of truth, and +the release workflow refuses any tag that disagrees with it. A release +candidate carries the version it is a candidate for, so `v2.1.0-rc1` also +requires `appVersion: "2.1.0"`. + +Two other version numbers are deliberately independent: + +- `helm/codeapi/Chart.yaml`'s `version` is the **chart** version. Bump it when + the chart's templates or values change, not when the app changes. It names + the packaged chart attached to the release (`codeapi-.tgz`). +- `service/package.json`'s `version` tracks the Lambda service package alone. + +By convention `api/package.json`'s `version` is kept in step with `appVersion`, +so the API package and the tag agree. Nothing enforces it. + +## Cutting a release + +1. Land the `appVersion` bump on `main` first. `main` takes no direct pushes + (see [CONTRIBUTING.md](../CONTRIBUTING.md)), so it arrives through a sync + pull request from the internal monorepo or a community pull request. Bump + the chart `version` too if the chart changed. +2. Run the **Release** workflow from the Actions tab against `main`, entering + the version (`v2.1.0`). Tick *draft* to review the generated notes before + they go public. + +The workflow validates the version, packages the Helm chart, then creates the +annotated tag and publishes the release. Packaging runs before tagging so a +failure — a rate-limited subchart pull, most likely — leaves the version +unused and the run safe to retry. + +A tag pushed by hand works as well, and takes the same path from validation +onward: + +```bash +git checkout main && git pull +git tag -a v2.1.0 -m v2.1.0 +git push origin v2.1.0 +``` + +## What the release contains + +- The tag, so a deployment can pin a commit. +- Notes: a preamble on pinning and installing, followed by the merged + pull requests since the previous tag, categorised per + [.github/release.yml](../.github/release.yml). +- `codeapi-.tgz`, the packaged Helm chart with its Redis and + MinIO subcharts vendored, so it installs without adding chart repositories. + +Release candidates are marked as pre-releases. The *Latest* badge moves only +when the release is stable **and** is the highest stable version in the +repository, so re-cutting an older patch cannot drag it backwards. + +## If a release goes wrong + +Delete the release and its tag, then re-run the workflow: + +```bash +gh release delete v2.1.0 --cleanup-tag --yes +``` + +Republishing the same version is only safe while nobody has deployed it. Once +a tag is public, ship a new patch instead. From 50368e0365ae7add8f8148b3d351563433a7e97a Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 10:45:37 -0400 Subject: [PATCH 2/8] =?UTF-8?q?=F0=9F=9B=96=20feat:=20Add=20Local=20NsJail?= =?UTF-8?q?=20Runtime=20Profile=20(#78)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add local NsJail runtime profile * fix: refresh stale local runtimes * fix: surface local runtime state loss --- api/Dockerfile | 17 +++ packages/code/README.md | 46 +++++- packages/code/src/cli.test.ts | 75 +++++++++- packages/code/src/cli.ts | 61 +++++++- packages/code/src/runtime.test.ts | 211 ++++++++++++++++++++++++++- packages/code/src/runtime.ts | 233 +++++++++++++++++++++++++----- 6 files changed, 593 insertions(+), 50 deletions(-) diff --git a/api/Dockerfile b/api/Dockerfile index 3526e98e..61a0bd08 100644 --- a/api/Dockerfile +++ b/api/Dockerfile @@ -171,6 +171,23 @@ ENV PORT=8080 \ EXPOSE 8080/tcp ENTRYPOINT ["/sandbox_api/entrypoint.sh"] +# Local direct-NsJail runtime used by @librechat/code's Docker supervisor. +# Runtime packages are mounted read-only at /pkgs, matching docker-compose.mac. +# This profile shares the Docker Desktop VM kernel and is for operator-trusted +# local/BYOM development; production untrusted execution should retain the +# separate MicroVM boundary described in the repository security guidance. +FROM sandbox-build AS local-oci-runtime + +ENV PORT=2000 \ + SANDBOX_PACKAGES_DIRECTORY=/pkgs \ + SANDBOX_OUTPUT_MAX_SIZE=65536 \ + SANDBOX_SESSION_WORKSPACE_ENABLED=true \ + SANDBOX_USE_CGROUPV2=false \ + SANDBOX_REMOVE_UMOUNT_AFTER_STARTUP=false + +EXPOSE 2000/tcp +ENTRYPOINT ["/sandbox_api/entrypoint.sh"] + # ============================================================================ # Stage 3: Build the Rust launcher binary (Fedora for libkrun ABI) # ============================================================================ diff --git a/packages/code/README.md b/packages/code/README.md index 3bbea73c..8e6c63b4 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -46,14 +46,14 @@ sets `no-new-privileges`. The trusted worker invokes the runner only through `docker exec` to `127.0.0.1` inside that container. The sandbox therefore has neither an inbound host port nor network egress. -It is exported for use by a deployment-specific worker launcher. It requires a +It requires a runtime image that provides the Code Interpreter `/api/v2/health` and `/api/v2/execute` endpoints and supports -`SANDBOX_SESSION_WORKSPACE_ENABLED=true`. A dedicated LibreChat runtime image -and CLI selector are the next layer; this adapter intentionally does not turn -an arbitrary image into a supported security boundary. Image-specific Linux -capabilities must be explicitly configured by the trusted launcher; the -default grants none. +`SANDBOX_SESSION_WORKSPACE_ENABLED=true`. The repository's +`local-oci-runtime` target supplies that API for the direct-NsJail macOS +profile. This adapter intentionally does not turn an arbitrary image into a +supported security boundary. Image-specific Linux capabilities must be +explicitly configured by the trusted launcher; the default grants none. To enable it from the bundled CLI, the host must give the worker access to its local Docker daemon and explicitly select a known runtime image: @@ -70,6 +70,40 @@ runtime image ships. Docker mode never binds a runner port on the VM. Do not mount the Docker socket into the sandbox; only the trusted worker may control the daemon. +For local Docker Desktop development, build the direct-NsJail target and use +the same capability and seccomp policy as `docker-compose.mac.yml`: + +```bash +docker build --target local-oci-runtime \ + -t librechat-code-runtime:local -f api/Dockerfile . + +LIBRECHAT_CODE_RUNTIME_SUPERVISOR=docker-macos-nsjail \ +LIBRECHAT_CODE_RUNTIME_IMAGE=librechat-code-runtime:local \ +LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE=./seccomp/nsjail.json \ +LIBRECHAT_CODE_DOCKER_PACKAGES_PATH=./data/pkgs \ +LIBRECHAT_CODE_STATEFUL_WORKSPACE=true \ +librechat-code run +``` + +The packages directory must already be populated using the repository's +package-init workflow. The worker mounts it read-only into each runtime. +Changing the image, package path, capabilities, seccomp contents, or other +confinement settings discards any surviving session container; the current +assignment fails explicitly so the lost workspace is never +presented as continuous state. Likewise, Docker Desktop remounts a fresh tmpfs +when this container restarts, so the profile discards a stopped container and +reports state loss instead of restarting it. The next assignment starts a new +environment. Treat profile changes and Docker restarts as environment resets +and preserve any needed workspace contents first. + +This first local profile supports inline request files. By-reference inputs and +generated-file uploads require a worker-mediated file relay and are not yet +supported; the runtime remains networkless rather than opening general egress +to reach a file server. +Direct NsJail shares the Docker Desktop VM kernel and is suitable for local or +operator-trusted development. Use a separate VM or MicroVM boundary for +internet-facing execution of code from untrusted users. + ## Static compatibility mode Non-hardened development deployments may still run with a static token: diff --git a/packages/code/src/cli.test.ts b/packages/code/src/cli.test.ts index 4684775f..82701e42 100644 --- a/packages/code/src/cli.test.ts +++ b/packages/code/src/cli.test.ts @@ -67,7 +67,7 @@ test('CLI rejects an unknown runtime supervisor before entering the run loop', ( assert.notEqual(result.status, 0); assert.match( result.stderr, - /LIBRECHAT_CODE_RUNTIME_SUPERVISOR must be either endpoint or docker/, + /LIBRECHAT_CODE_RUNTIME_SUPERVISOR must be endpoint, docker, or docker-macos-nsjail/, ); }); @@ -90,3 +90,76 @@ test('CLI requires a runtime image for Docker supervision', () => { assert.notEqual(result.status, 0); assert.match(result.stderr, /LIBRECHAT_CODE_RUNTIME_IMAGE is required/); }); + +test('CLI requires the macOS NsJail seccomp profile', () => { + const result = spawnSync( + process.execPath, + [fileURLToPath(new URL('./cli.js', import.meta.url))], + { + encoding: 'utf8', + env: { + ...process.env, + LIBRECHAT_CODE_URL: 'https://code.example/v1', + LIBRECHAT_CODE_WORKER_TOKEN: 'worker-secret', + LIBRECHAT_CODE_WORKER_ID: 'engineering-vm', + LIBRECHAT_CODE_RUNTIME_SUPERVISOR: 'docker-macos-nsjail', + LIBRECHAT_CODE_RUNTIME_IMAGE: 'example/runtime:latest', + }, + }, + ); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE is required/); +}); + +test('CLI requires a package mount for the macOS NsJail profile', () => { + const result = spawnSync( + process.execPath, + [fileURLToPath(new URL('./cli.js', import.meta.url))], + { + encoding: 'utf8', + env: { + ...process.env, + LIBRECHAT_CODE_URL: 'https://code.example/v1', + LIBRECHAT_CODE_WORKER_TOKEN: 'worker-secret', + LIBRECHAT_CODE_WORKER_ID: 'engineering-vm', + LIBRECHAT_CODE_RUNTIME_SUPERVISOR: 'docker-macos-nsjail', + LIBRECHAT_CODE_RUNTIME_IMAGE: 'example/runtime:latest', + LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE: './seccomp/nsjail.json', + }, + }, + ); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /LIBRECHAT_CODE_DOCKER_PACKAGES_PATH is required/); +}); + +test('CLI reset does not require Docker runtime launch inputs', () => { + const result = spawnSync( + process.execPath, + [ + fileURLToPath(new URL('./cli.js', import.meta.url)), + 'reset-workspace', + 'runtime-session-1', + ], + { + encoding: 'utf8', + env: { + ...process.env, + LIBRECHAT_CODE_URL: 'http://127.0.0.1:1/v1', + LIBRECHAT_CODE_WORKER_TOKEN: 'worker-secret', + LIBRECHAT_CODE_WORKER_ID: 'engineering-vm', + LIBRECHAT_CODE_RUNTIME_SUPERVISOR: 'docker-macos-nsjail', + LIBRECHAT_CODE_RUNTIME_IMAGE: undefined, + LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE: undefined, + LIBRECHAT_CODE_DOCKER_PACKAGES_PATH: undefined, + }, + }, + ); + + assert.notEqual(result.status, 0); + assert.doesNotMatch( + result.stderr, + /LIBRECHAT_CODE_(?:RUNTIME_IMAGE|DOCKER_SECCOMP_PROFILE|DOCKER_PACKAGES_PATH) is required/, + ); +}); diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index f6fb3fb1..457af16f 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -1,5 +1,7 @@ #!/usr/bin/env node import { createHash } from 'node:crypto'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; import { pairBridgeWorker } from './pairing.js'; import { @@ -29,6 +31,23 @@ function list(value: string | undefined): string[] { ); } +const MACOS_NSJAIL_CAPABILITIES = [ + 'SYS_ADMIN', + 'SYS_CHROOT', + 'SYS_PTRACE', + 'SETUID', + 'SETGID', + 'NET_ADMIN', + 'DAC_OVERRIDE', + 'DAC_READ_SEARCH', + 'CHOWN', + 'FOWNER', + 'FSETID', + 'KILL', + 'SETFCAP', + 'MKNOD', +]; + function option(args: string[], name: string): string | undefined { const index = args.indexOf(name); if (index >= 0) return args[index + 1]; @@ -88,9 +107,13 @@ async function run(runtimeSessionId?: string): Promise { 'true'; const runtimeMode = process.env.LIBRECHAT_CODE_RUNTIME_SUPERVISOR?.trim().toLowerCase() ?? 'endpoint'; - if (runtimeMode !== 'endpoint' && runtimeMode !== 'docker') { + if ( + runtimeMode !== 'endpoint' && + runtimeMode !== 'docker' && + runtimeMode !== 'docker-macos-nsjail' + ) { throw new Error( - 'LIBRECHAT_CODE_RUNTIME_SUPERVISOR must be either endpoint or docker', + 'LIBRECHAT_CODE_RUNTIME_SUPERVISOR must be endpoint, docker, or docker-macos-nsjail', ); } const sandboxEndpoint = @@ -116,7 +139,7 @@ async function run(runtimeSessionId?: string): Promise { statefulWorkspace, sandboxProfile: process.env.LIBRECHAT_CODE_SANDBOX_PROFILE ?? - (runtimeMode === 'docker' ? 'oci-docker' : 'nsjail'), + (runtimeMode.startsWith('docker') ? 'oci-docker' : 'nsjail'), runtimes: list(process.env.LIBRECHAT_CODE_RUNTIMES), policyDigest: createHash('sha256').update(policy).digest('hex'), }; @@ -134,12 +157,42 @@ async function run(runtimeSessionId?: string): Promise { identity: workerIdentity, workerId, runtimeSupervisor: - runtimeMode === 'docker' + runtimeMode !== 'endpoint' ? new DockerRuntimeSupervisor({ image: runtimeSessionId == null ? required('LIBRECHAT_CODE_RUNTIME_IMAGE') : process.env.LIBRECHAT_CODE_RUNTIME_IMAGE?.trim(), + ...(runtimeMode === 'docker-macos-nsjail' && runtimeSessionId == null + ? (() => { + const seccompProfile = resolve( + required('LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE'), + ); + const packagesPath = resolve( + required('LIBRECHAT_CODE_DOCKER_PACKAGES_PATH'), + ); + return { + capabilities: MACOS_NSJAIL_CAPABILITIES, + securityOptions: [`seccomp=${seccompProfile}`], + profileRevision: createHash('sha256') + .update(readFileSync(seccompProfile)) + .digest('hex'), + restartStoppedContainers: false, + bindMounts: [ + { + source: packagesPath, + target: '/pkgs', + readOnly: true, + }, + ], + httpClient: 'bun', + environment: { + SANDBOX_USE_CGROUPV2: 'false', + SANDBOX_REMOVE_UMOUNT_AFTER_STARTUP: 'false', + }, + }; + })() + : {}), }) : new EndpointRuntimeSupervisor({ endpoint: sandboxEndpoint, diff --git a/packages/code/src/runtime.test.ts b/packages/code/src/runtime.test.ts index 9963efc1..113f93bb 100644 --- a/packages/code/src/runtime.test.ts +++ b/packages/code/src/runtime.test.ts @@ -109,6 +109,54 @@ test('docker runtime supervisor creates a networkless stateful runtime and execu assert.ok(health?.includes('--max-time')); }); +test('docker runtime supervisor applies an explicit macOS NsJail confinement profile', async () => { + const calls: string[][] = []; + const client: ContainerRuntimeClient = { + async run(args) { + calls.push(args); + if (args[0] === 'container' && args[1] === 'inspect') throw new Error('No such container'); + if (args[0] === 'run') return 'container-id\n'; + if (args[0] === 'exec') return '200'; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerRuntimeSupervisor({ + image: 'example/code-runtime:latest', + client, + capabilities: ['SYS_ADMIN', 'CHOWN'], + securityOptions: ['seccomp=/repo/seccomp/nsjail.json'], + environment: { SANDBOX_USE_CGROUPV2: 'false' }, + bindMounts: [{ source: '/repo/data/pkgs', target: '/pkgs', readOnly: true }], + httpClient: 'bun', + }); + + await supervisor.acquire(assignment('rt-user-1')); + + const run = calls.find(args => args[0] === 'run') ?? []; + assert.ok(run.includes('SYS_ADMIN')); + assert.ok(run.includes('CHOWN')); + assert.ok(run.includes('seccomp=/repo/seccomp/nsjail.json')); + assert.ok(run.includes('SANDBOX_USE_CGROUPV2=false')); + assert.ok(run.includes('type=bind,source=/repo/data/pkgs,target=/pkgs,readonly')); + assert.ok( + run.indexOf('SANDBOX_USE_CGROUPV2=false') < + run.indexOf('SANDBOX_SESSION_WORKSPACE_ENABLED=true'), + ); + const health = calls.find(args => args[0] === 'exec') ?? []; + assert.ok(health.includes('bun')); +}); + +test('docker runtime supervisor rejects relative bind mount paths', () => { + assert.throws( + () => + new DockerRuntimeSupervisor({ + image: 'example/code-runtime:latest', + bindMounts: [{ source: './data/pkgs', target: '/pkgs', readOnly: true }], + }), + /absolute comma-free sources and targets/, + ); +}); + test('docker runtime supervisor rejects malformed runtime response framing', async () => { const client: ContainerRuntimeClient = { async run(args) { @@ -130,10 +178,23 @@ test('docker runtime supervisor rejects malformed runtime response framing', asy test('docker runtime supervisor preserves an existing stateful container after a health failure', async () => { const calls: string[][] = []; + let profileDigest: string | undefined; + let healthChecks = 0; const client: ContainerRuntimeClient = { async run(args) { calls.push(args); - if (args[0] === 'container' && args[1] === 'inspect') return 'true\n'; + if (args[0] === 'container' && args[1] === 'inspect') { + if (!profileDigest) throw new Error('No such container'); + return `true|${profileDigest}|sha256:image-1\n`; + } + if (args[0] === 'image' && args[1] === 'inspect') return 'sha256:image-1\n'; + if (args[0] === 'run') { + profileDigest = args + .find(value => value.startsWith('com.librechat.code.profile-digest=')) + ?.split('=')[1]; + return 'container-id\n'; + } + if (args[0] === 'exec' && healthChecks++ === 0) return '200'; if (args[0] === 'exec') throw new Error('runner unavailable'); throw new Error(`Unexpected Docker command: ${args.join(' ')}`); }, @@ -144,10 +205,158 @@ test('docker runtime supervisor preserves an existing stateful container after a startupTimeoutMs: 1, }); + await supervisor.acquire(assignment('rt-user-1')); await assert.rejects(supervisor.acquire(assignment('rt-user-1')), /did not become healthy/); assert.equal(calls.some(args => args[0] === 'container' && args[1] === 'rm'), false); }); +test('docker runtime supervisor reports state loss before recreating after profile drift', async () => { + const calls: string[][] = []; + let containerExists = true; + const client: ContainerRuntimeClient = { + async run(args) { + calls.push(args); + if (args[0] === 'container' && args[1] === 'inspect') { + if (!containerExists) throw new Error('No such container'); + return 'true|stale-profile|sha256:image-1\n'; + } + if (args[0] === 'image' && args[1] === 'inspect') return 'sha256:image-1\n'; + if (args[0] === 'container' && args[1] === 'rm') { + containerExists = false; + return 'removed\n'; + } + if (args[0] === 'run') { + containerExists = true; + return 'container-id\n'; + } + if (args[0] === 'exec') return '200'; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerRuntimeSupervisor({ + image: 'example/code-runtime:latest', + profileRevision: 'seccomp-v2', + client, + }); + + await assert.rejects( + supervisor.acquire(assignment('rt-user-1')), + /workspace was discarded because its confinement profile or image changed/, + ); + await supervisor.acquire(assignment('rt-user-1')); + + const removalIndex = calls.findIndex(args => args[0] === 'container' && args[1] === 'rm'); + const creationIndex = calls.findIndex(args => args[0] === 'run'); + assert.ok(removalIndex >= 0); + assert.ok(creationIndex > removalIndex); + assert.ok( + calls[creationIndex]?.some(value => + value.startsWith('com.librechat.code.profile-digest='), + ), + ); +}); + +test('docker runtime supervisor reports state loss before recreating after an image tag moves', async () => { + const calls: string[][] = []; + let profileDigest: string | undefined; + let containerExists = false; + let currentImageId = 'sha256:image-1'; + const client: ContainerRuntimeClient = { + async run(args) { + calls.push(args); + if (args[0] === 'container' && args[1] === 'inspect') { + if (!containerExists) throw new Error('No such container'); + return `true|${profileDigest}|sha256:image-1\n`; + } + if (args[0] === 'image' && args[1] === 'inspect') { + return `${currentImageId}\n`; + } + if (args[0] === 'container' && args[1] === 'rm') { + containerExists = false; + return 'removed\n'; + } + if (args[0] === 'run') { + profileDigest = args + .find(value => value.startsWith('com.librechat.code.profile-digest=')) + ?.split('=')[1]; + containerExists = true; + return 'container-id\n'; + } + if (args[0] === 'exec') return '200'; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerRuntimeSupervisor({ + image: 'example/code-runtime:latest', + client, + }); + + await supervisor.acquire(assignment('rt-user-1')); + currentImageId = 'sha256:image-2'; + await assert.rejects( + supervisor.acquire(assignment('rt-user-1')), + /workspace was discarded because its confinement profile or image changed/, + ); + await supervisor.acquire(assignment('rt-user-1')); + + assert.equal( + calls.filter(args => args[0] === 'container' && args[1] === 'rm').length, + 1, + ); + assert.equal(calls.filter(args => args[0] === 'run').length, 2); +}); + +test('docker runtime supervisor reports state loss instead of restarting tmpfs sessions', async () => { + const calls: string[][] = []; + let profileDigest: string | undefined; + let containerExists = false; + let running = false; + const client: ContainerRuntimeClient = { + async run(args) { + calls.push(args); + if (args[0] === 'container' && args[1] === 'inspect') { + if (!containerExists) throw new Error('No such container'); + return `${running}|${profileDigest}|sha256:image-1\n`; + } + if (args[0] === 'image' && args[1] === 'inspect') return 'sha256:image-1\n'; + if (args[0] === 'container' && args[1] === 'rm') { + containerExists = false; + return 'removed\n'; + } + if (args[0] === 'run') { + profileDigest = args + .find(value => value.startsWith('com.librechat.code.profile-digest=')) + ?.split('=')[1]; + containerExists = true; + running = true; + return 'container-id\n'; + } + if (args[0] === 'exec') return '200'; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerRuntimeSupervisor({ + image: 'example/code-runtime:latest', + restartStoppedContainers: false, + client, + }); + + await supervisor.acquire(assignment('rt-user-1')); + running = false; + await assert.rejects( + supervisor.acquire(assignment('rt-user-1')), + /workspace was discarded because its container stopped/, + ); + await supervisor.acquire(assignment('rt-user-1')); + + assert.equal(calls.filter(args => args[0] === 'start').length, 0); + assert.equal( + calls.filter(args => args[0] === 'container' && args[1] === 'rm').length, + 1, + ); + assert.equal(calls.filter(args => args[0] === 'run').length, 2); +}); + test('docker runtime supervisor propagates removal failures and forwards reset cancellation', async () => { const controller = new AbortController(); let receivedSignal: AbortSignal | undefined; diff --git a/packages/code/src/runtime.ts b/packages/code/src/runtime.ts index 89eaca95..bde50322 100644 --- a/packages/code/src/runtime.ts +++ b/packages/code/src/runtime.ts @@ -45,7 +45,13 @@ export interface ContainerRuntimeRunOptions { export interface DockerRuntimeSupervisorOptions { image?: string; + profileRevision?: string; + restartStoppedContainers?: boolean; capabilities?: string[]; + securityOptions?: string[]; + environment?: Record; + bindMounts?: DockerRuntimeBindMount[]; + httpClient?: 'curl' | 'bun'; dockerCommand?: string; runnerPort?: number; startupTimeoutMs?: number; @@ -53,6 +59,18 @@ export interface DockerRuntimeSupervisorOptions { client?: ContainerRuntimeClient; } +export interface DockerRuntimeBindMount { + source: string; + target: string; + readOnly?: boolean; +} + +interface DockerContainerState { + running: boolean; + profileDigest?: string; + imageId?: string; +} + const DEFAULT_RUNNER_PORT = 2000; const DEFAULT_STARTUP_TIMEOUT_MS = 30_000; const DEFAULT_HEALTH_PATH = '/api/v2/health'; @@ -83,6 +101,11 @@ function isMissingContainerError(error: unknown): boolean { return /(?:no such container|no such object)/i.test(error.message); } +function isMissingImageError(error: unknown): boolean { + if (!(error instanceof Error)) return false; + return /(?:no such image|no such object)/i.test(error.message); +} + class DockerCliClient implements ContainerRuntimeClient { private readonly command: string; @@ -137,6 +160,11 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { private readonly startupTimeoutMs: number; private readonly healthPath: string; private readonly capabilities: string[]; + private readonly securityOptions: string[]; + private readonly environment: Record; + private readonly bindMounts: DockerRuntimeBindMount[]; + private readonly httpClient: 'curl' | 'bun'; + private readonly restartStoppedContainers: boolean; constructor(private readonly options: DockerRuntimeSupervisorOptions) { if (options.image != null && options.image.trim().length === 0) { @@ -149,7 +177,23 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { this.runnerPort = options.runnerPort ?? DEFAULT_RUNNER_PORT; this.startupTimeoutMs = options.startupTimeoutMs ?? DEFAULT_STARTUP_TIMEOUT_MS; this.healthPath = options.healthPath ?? DEFAULT_HEALTH_PATH; - this.capabilities = options.capabilities ?? []; + this.capabilities = [...(options.capabilities ?? [])]; + this.securityOptions = [...(options.securityOptions ?? [])]; + this.environment = { ...options.environment }; + this.bindMounts = (options.bindMounts ?? []).map((mount) => ({ ...mount })); + this.httpClient = options.httpClient ?? 'curl'; + this.restartStoppedContainers = options.restartStoppedContainers ?? true; + if ( + this.bindMounts.some( + ({ source, target }) => + !source.startsWith('/') || + !target.startsWith('/') || + source.includes(',') || + target.includes(','), + ) + ) { + throw new Error('Docker runtime bind mounts require absolute comma-free sources and targets'); + } } async acquire(assignment: BridgeAssignment, signal?: AbortSignal): Promise { @@ -158,7 +202,12 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { const name = containerName(sessionId); let created = false; try { - created = await this.ensureContainer(name, sessionId, signal); + created = await this.ensureContainer( + name, + sessionId, + assignment.runtimeSessionId != null, + signal, + ); await this.waitForHealth(name, signal); return { sessionId, @@ -186,13 +235,36 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { private async ensureContainer( name: string, runtimeSessionId: string, + stateful: boolean, signal?: AbortSignal, ): Promise { const image = this.options.image?.trim(); if (!image) throw new Error('Docker runtime image is required for acquisition'); - const running = await this.containerRunning(name, signal); - if (running) return false; - if (running === false) { + const profileDigest = this.profileDigest(image); + let state = await this.containerState(name, signal); + if (state != null) { + const currentImageId = await this.imageId(image, signal); + if ( + state.profileDigest !== profileDigest || + (currentImageId != null && state.imageId !== currentImageId) + ) { + await this.remove(name, signal); + state = undefined; + if (stateful) { + throw new Error( + 'Docker runtime workspace was discarded because its confinement profile or image changed', + ); + } + } + } + if (state?.running) return false; + if (state != null) { + if (!this.restartStoppedContainers) { + await this.remove(name, signal); + throw new Error( + 'Docker runtime workspace was discarded because its container stopped', + ); + } await this.client.run(['start', name], { signal }); return false; } @@ -209,10 +281,18 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { ...this.capabilities.flatMap((capability) => ['--cap-add', capability]), '--security-opt', 'no-new-privileges:true', + ...this.securityOptions.flatMap((option) => ['--security-opt', option]), + ...this.bindMounts.flatMap(({ source, target, readOnly }) => [ + '--mount', + `type=bind,source=${source},target=${target}${readOnly ? ',readonly' : ''}`, + ]), '--label', 'com.librechat.code.runtime=true', '--label', `com.librechat.code.runtime-hash=${containerSuffix(runtimeSessionId)}`, + '--label', + `com.librechat.code.profile-digest=${profileDigest}`, + ...Object.entries(this.environment).flatMap(([name, value]) => ['--env', `${name}=${value}`]), '--env', 'SANDBOX_SESSION_WORKSPACE_ENABLED=true', image, @@ -222,19 +302,73 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { return true; } - private async containerRunning(name: string, signal?: AbortSignal): Promise { + private profileDigest(image: string): string { + return createHash('sha256') + .update( + JSON.stringify({ + version: 1, + image, + profileRevision: this.options.profileRevision ?? null, + restartStoppedContainers: this.restartStoppedContainers, + capabilities: this.capabilities, + securityOptions: this.securityOptions, + environment: Object.entries(this.environment).sort(([left], [right]) => + left.localeCompare(right), + ), + bindMounts: this.bindMounts, + }), + ) + .digest('hex'); + } + + private async containerState( + name: string, + signal?: AbortSignal, + ): Promise { try { const value = await this.client.run( - ['container', 'inspect', '--format', '{{.State.Running}}', name], + [ + 'container', + 'inspect', + '--format', + '{{.State.Running}}|{{index .Config.Labels "com.librechat.code.profile-digest"}}|{{.Image}}', + name, + ], { signal }, ); - return value.trim() === 'true'; + const [running, profileDigest, imageId] = value.trim().split('|'); + if (running !== 'true' && running !== 'false') { + throw new Error( + 'Docker runtime container inspection returned an invalid state', + ); + } + return { + running: running === 'true', + ...(profileDigest && profileDigest !== '' ? { profileDigest } : {}), + ...(imageId ? { imageId } : {}), + }; } catch (error) { if (isMissingContainerError(error)) return undefined; throw error; } } + private async imageId( + image: string, + signal?: AbortSignal, + ): Promise { + try { + const value = await this.client.run( + ['image', 'inspect', '--format', '{{.Id}}', image], + { signal }, + ); + return value.trim() || undefined; + } catch (error) { + if (isMissingImageError(error)) return undefined; + throw error; + } + } + private async waitForHealth(name: string, signal?: AbortSignal): Promise { const deadline = Date.now() + this.startupTimeoutMs; let lastError: unknown; @@ -242,21 +376,32 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { if (signal?.aborted) throw signal.reason ?? new DOMException('aborted', 'AbortError'); try { const remainingMs = Math.max(1, deadline - Date.now()); + const healthUrl = `http://127.0.0.1:${this.runnerPort}${this.healthPath}`; const status = await this.client.run( - [ - 'exec', - name, - 'curl', - '--silent', - '--show-error', - '--max-time', - (remainingMs / 1000).toFixed(3), - '--output', - '/dev/null', - '--write-out', - '%{http_code}', - `http://127.0.0.1:${this.runnerPort}${this.healthPath}`, - ], + this.httpClient === 'bun' + ? [ + 'exec', + name, + 'bun', + '-e', + 'const r=await fetch(process.argv.at(-2),{signal:AbortSignal.timeout(Number(process.argv.at(-1)))});process.stdout.write(String(r.status));', + healthUrl, + String(remainingMs), + ] + : [ + 'exec', + name, + 'curl', + '--silent', + '--show-error', + '--max-time', + (remainingMs / 1000).toFixed(3), + '--output', + '/dev/null', + '--write-out', + '%{http_code}', + healthUrl, + ], { signal }, ); if (status.trim() === '200') return; @@ -281,23 +426,35 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { throw new Error('Runtime request headers cannot contain line breaks'); } const marker = randomBytes(32).toString('hex'); + const executeUrl = `http://127.0.0.1:${this.runnerPort}/api/v2/execute`; const output = await this.client.run( - [ - 'exec', - '--interactive', - name, - 'curl', - '--silent', - '--show-error', - '--request', - 'POST', - ...Object.entries(request.headers).flatMap(([name, value]) => ['--header', `${name}: ${value}`]), - '--data-binary', - '@-', - '--write-out', - `\n${marker}%{http_code}`, - `http://127.0.0.1:${this.runnerPort}/api/v2/execute`, - ], + this.httpClient === 'bun' + ? [ + 'exec', + '--interactive', + name, + 'bun', + '-e', + `const b=await Bun.stdin.text();const r=await fetch(process.argv.at(-2),{method:'POST',headers:JSON.parse(process.argv.at(-1)),body:b});process.stdout.write(await r.text());process.stdout.write('\\n${marker}'+r.status);`, + executeUrl, + JSON.stringify(request.headers), + ] + : [ + 'exec', + '--interactive', + name, + 'curl', + '--silent', + '--show-error', + '--request', + 'POST', + ...Object.entries(request.headers).flatMap(([name, value]) => ['--header', `${name}: ${value}`]), + '--data-binary', + '@-', + '--write-out', + `\n${marker}%{http_code}`, + executeUrl, + ], { input: request.body, signal: request.signal }, ); const suffix = new RegExp(`\\n${marker}(\\d{3})$`); From 6967ba947cae5b38cee196798e5145ad71f76d96 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 10:47:15 -0400 Subject: [PATCH 3/8] =?UTF-8?q?=F0=9F=8E=B4=20fix:=20Rotate=20PTC=20Replay?= =?UTF-8?q?=20Client=20Tokens=20(#82)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(lambda-microvm): give each PTC replay iteration a distinct clientToken PTC replay reuses one executionId across every stateless iteration, but the launch clientToken was derived from that executionId alone. Each iteration relaunches with a changed sandbox payload (a fresh _ptc_history.json), so AWS rejected the second launch with "The provided clientToken was used with different request parameters" and LibreChat surfaced the generic "Bash programmatic execution failed" (#59). Fold the launch inputs and the per-iteration request body into the token so each distinct launch gets a distinct token while an identical retry stays idempotent. Reuses runtimeSessionLaunchRequestFingerprint rather than restating the launch inputs. Reported with a working patch by @snapydziuba. * fix(lambda-microvm): key the stateless launch token to the queued job Addresses codex review on #82. Hashing the request body made the token move between attempts of the same job: workers.ts rebuilds the request on every attempt with a fresh egress grant (random IV and sandbox session id) and a re-signed manifest, so a replacement worker taking over a stalled job would derive a different token, launch a second VM, and leave the accepted one burning capacity until its maximum duration expired. Use the queued job id instead. Each PTC replay iteration is enqueued as its own job, so it is distinct per iteration and stable across attempts of the same job -- and it carries no capability-bearing material. The launch configuration stays in the digest so a worker with a different config cannot reuse another's token. --- .../sandbox-backend/lambda-microvm.test.ts | 54 +++++++++++++++++-- service/src/sandbox-backend/lambda-microvm.ts | 51 +++++++++++++++++- service/src/sandbox-backend/types.ts | 8 +++ service/src/workers.ts | 1 + 4 files changed, 109 insertions(+), 5 deletions(-) diff --git a/service/src/sandbox-backend/lambda-microvm.test.ts b/service/src/sandbox-backend/lambda-microvm.test.ts index 6641fa6d..02110edd 100644 --- a/service/src/sandbox-backend/lambda-microvm.test.ts +++ b/service/src/sandbox-backend/lambda-microvm.test.ts @@ -26,6 +26,7 @@ import { runtimeSessionLaunchClientToken, runtimeSessionLaunchFingerprint, runtimeSessionLaunchGenerationSeed, + statelessLaunchClientToken, type LambdaMicrovmBackendConfig, } from './lambda-microvm'; import { SandboxBackendError } from './types'; @@ -301,6 +302,50 @@ describe('runtime session launch tokens', () => { }); }); +describe('statelessLaunchClientToken', () => { + test('is deterministic for an identical relaunch', () => { + expect(statelessLaunchClientToken('exec_42', config(), 420, 'job_1')) + .toBe(statelessLaunchClientToken('exec_42', config(), 420, 'job_1')); + }); + + /* PTC replay reuses one executionId across iterations, each enqueued as its + * own job. A token derived from the executionId alone repeated, and AWS + * rejected the relaunch with "The provided clientToken was used with + * different request parameters". */ + test('differs per replay iteration', () => { + const round1 = statelessLaunchClientToken('exec_42', config(), 420, 'job_1'); + const round2 = statelessLaunchClientToken('exec_42', config(), 420, 'job_2'); + expect(round1).not.toBe(round2); + expect(round1).toMatch(/^exec-exec_42-[0-9a-f]{16}$/); + expect(round2).toMatch(/^exec-exec_42-[0-9a-f]{16}$/); + }); + + /* A replacement worker taking over a stalled job rebuilds the request with a + * fresh egress grant, sandbox session id and re-signed manifest. The token + * must not move with it, or RunMicrovm idempotency cannot recover a launch + * AWS already accepted and the orphaned VM burns capacity until it expires. */ + test('is stable across attempts of the same queued job', () => { + const firstAttempt = statelessLaunchClientToken('exec_42', config(), 420, 'job_1'); + const stalledRetry = statelessLaunchClientToken('exec_42', config(), 420, 'job_1'); + expect(stalledRetry).toBe(firstAttempt); + }); + + test('differs when launch configuration or duration changes', () => { + const base = statelessLaunchClientToken('exec_42', config(), 420, 'job_1'); + expect(statelessLaunchClientToken('exec_42', config({ imageVersion: '4' }), 420, 'job_1')) + .not.toBe(base); + expect(statelessLaunchClientToken('exec_42', config(), 421, 'job_1')).not.toBe(base); + }); + + test('stays within the AWS clientToken budget including the retry suffix', () => { + const token = statelessLaunchClientToken('exec_42', config(), 420, 'job_1'); + expect(`${token}-r1`.length).toBeLessThanOrEqual(128); + expect(() => statelessLaunchClientToken('e'.repeat(200), config(), 420, 'job_1')).toThrow( + 'Stateless launch clientToken exceeds the AWS length limit', + ); + }); +}); + describe('LambdaMicrovmSandboxBackend stateless execution', () => { test('run -> health -> execute -> terminate happy path', async () => { const fake = fakeClient(); @@ -315,7 +360,7 @@ describe('LambdaMicrovmSandboxBackend stateless execution', () => { expect(runCalls).toHaveLength(1); const runArgs = runCalls[0].args as { imageIdentifier: string; clientToken?: string; maximumDurationSeconds: number }; expect(runArgs.imageIdentifier).toBe('arn:aws:lambda:us-east-2:1:microvm-image:codeapi'); - expect(runArgs.clientToken).toBe('exec-exec_42'); + expect(runArgs.clientToken).toMatch(/^exec-exec_42-[0-9a-f]{16}$/); expect(runArgs.maximumDurationSeconds).toBe(Math.ceil(300_000 / 1_000) + 120); const executeReq = captured.find((c) => c.path === '/api/v2/execute'); @@ -509,8 +554,8 @@ describe('LambdaMicrovmSandboxBackend stateless execution', () => { const runCalls = fake.callsFor('runMicrovm'); expect(runCalls).toHaveLength(2); const tokens = runCalls.map((call) => (call.args as { clientToken?: string }).clientToken); - expect(tokens[0]).toBe('exec-exec_42'); - expect(tokens[1]).toBe('exec-exec_42-r1'); + expect(tokens[0]).toMatch(/^exec-exec_42-[0-9a-f]{16}$/); + expect(tokens[1]).toBe(`${tokens[0]}-r1`); }); test('the boot-death retry consumes only the first attempt remaining launch budget', async () => { @@ -533,7 +578,8 @@ describe('LambdaMicrovmSandboxBackend stateless execution', () => { }); const tokens = fake.callsFor('runMicrovm') .map(call => (call.args as { clientToken?: string }).clientToken); - expect(tokens).toEqual(['exec-exec_42', 'exec-exec_42-r1']); + expect(tokens[0]).toMatch(/^exec-exec_42-[0-9a-f]{16}$/); + expect(tokens).toEqual([tokens[0], `${tokens[0]}-r1`]); expect(captured.some(request => request.path === '/api/v2/execute')).toBe(false); expect(fake.callsFor('terminateMicrovm')).toHaveLength(2); }); diff --git a/service/src/sandbox-backend/lambda-microvm.ts b/service/src/sandbox-backend/lambda-microvm.ts index 1cb8a4dd..d947c76e 100644 --- a/service/src/sandbox-backend/lambda-microvm.ts +++ b/service/src/sandbox-backend/lambda-microvm.ts @@ -138,6 +138,48 @@ export function runtimeSessionLaunchGenerationSeed(config: LambdaMicrovmBackendC return RUNTIME_SESSION_NAMESPACED_GENERATION_MIN + offset; } +/** Stateless one-shot launch token. + * + * PTC replay reuses one executionId across every iteration, so a token derived + * from the executionId alone repeats while the launch parameters change with + * each iteration's payload. AWS rejects that with "The provided clientToken was + * used with different request parameters" and the whole execution fails. + * + * The discriminator is the queued job id rather than the request body: the body + * is rebuilt with a fresh egress grant, sandbox session id and re-signed + * manifest on every job attempt, so hashing it would hand a replacement worker + * a different token after a stalled-job takeover and launch a second VM instead + * of recovering the accepted one through RunMicrovm idempotency. The job id is + * distinct per replay iteration and stable across attempts of the same job. + * + * The launch configuration stays in the digest because a worker whose config + * differs must not reuse another worker's token. */ +export function statelessLaunchClientToken( + executionId: string, + config: LambdaMicrovmBackendConfig, + maxDurationSeconds: number, + queuedJobId: string, +): string { + const suffix = createHash('sha256') + .update( + JSON.stringify({ + launchRequest: runtimeSessionLaunchRequestFingerprint(config), + maximumDurationSeconds: maxDurationSeconds, + queuedJobId, + }), + 'utf8', + ) + .digest('hex') + .slice(0, 16); + const token = `exec-${executionId}-${suffix}`; + /* launch() can add "-r1" after a clean boot-time death; reserve those three + * characters so both attempts stay within AWS's 128-byte limit. */ + if (token.length > 125) { + throw new Error('Stateless launch clientToken exceeds the AWS length limit'); + } + return token; +} + export function runtimeSessionLaunchClientToken(runtimeSessionId: string, generation: number): string { if (!Number.isSafeInteger(generation) || generation < 1) { throw new Error('Runtime session generation must be a positive safe integer'); @@ -259,7 +301,14 @@ export class LambdaMicrovmSandboxBackend implements SandboxBackend { Math.ceil(this.config.jobTimeoutMs / 1_000) + 120, ); const vm = await this.launch(client, ctx, { - clientToken: ctx.executionId !== '' ? `exec-${ctx.executionId}` : `exec-${nanoid()}`, + clientToken: statelessLaunchClientToken( + ctx.executionId !== '' ? ctx.executionId : nanoid(), + this.config, + maxDurationSeconds, + /* No queued job id (direct backend caller): fall back to a fresh value + * so distinct launches never collide on one token. */ + ctx.queuedJobId ?? nanoid(), + ), maxDurationSeconds, }); let terminateReason = 'stateless'; diff --git a/service/src/sandbox-backend/types.ts b/service/src/sandbox-backend/types.ts index 64e5b6e0..96151dde 100644 --- a/service/src/sandbox-backend/types.ts +++ b/service/src/sandbox-backend/types.ts @@ -39,6 +39,14 @@ export interface SandboxExecuteContext { canonicalUserId?: string; /** Trusted API-selected outbound worker. Presence requires a tenant-bound credential. */ bridgeWorkerId?: string; + /** Stable identifier for this queued iteration, used to derive an idempotent + * stateless launch token. PTC replay reuses one executionId across every + * iteration, so the executionId alone cannot separate them; the request body + * can, but it is rebuilt with a fresh egress grant and manifest on every job + * attempt, so hashing it would break RunMicrovm idempotency when BullMQ + * reprocesses a stalled job. The queued job id is distinct per iteration and + * stable across attempts of the same job. */ + queuedJobId?: string; /** Absent ⇒ stateless execution (no runtime session affinity). */ runtimeSessionId?: string; runtimeSessionMode: t.RuntimeSessionMode; diff --git a/service/src/workers.ts b/service/src/workers.ts index d2048dc8..215a4d1b 100644 --- a/service/src/workers.ts +++ b/service/src/workers.ts @@ -141,6 +141,7 @@ async function processJobInner(job: t.ExecuteJob): Promise { }, { executionId: job.data.executionId ?? '', + queuedJobId: job.id != null ? String(job.id) : undefined, language, isSynthetic: isSyntheticJob, signal: controller.signal, From 543bf4e4ad161c627ca1156d5036ca683b664aa6 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 10:49:26 -0400 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=93=9B=20fix:=20Preserve=20Uploaded?= =?UTF-8?q?=20Filenames=20Without=20Metadata=20(#79)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: preserve uploaded filenames without s3 metadata * fix: preserve filenames through hardened egress --- api/src/download.test.ts | 23 ++++++++++ api/src/job-helpers.test.ts | 27 ++++++++++++ api/src/job.ts | 17 +++++++- service/src/egress-gateway.test.ts | 24 ++++++++++- service/src/egress-gateway.ts | 17 ++++++-- service/src/file-metadata.test.ts | 68 ++++++++++++++++++++++++++++++ service/src/file-metadata.ts | 62 +++++++++++++++++++++++++++ service/src/file-server.ts | 49 ++++++--------------- 8 files changed, 245 insertions(+), 42 deletions(-) create mode 100644 service/src/file-metadata.test.ts create mode 100644 service/src/file-metadata.ts diff --git a/api/src/download.test.ts b/api/src/download.test.ts index 692957b2..76e21395 100644 --- a/api/src/download.test.ts +++ b/api/src/download.test.ts @@ -289,6 +289,29 @@ describe('downloadAndWriteFile / RFC 5987 round-trip', () => { expect(contents).toBe('legacy bytes'); }); + it('writes under the requested name when a legacy server returns an opaque storage filename', async () => { + const file: TFile = { + id: 'opaque-storage-id', + storage_session_id: 'prev-session', + name: 'Sample_-_Superstore.xlsx', + }; + routes.set(`/sessions/${encodeURIComponent(file.storage_session_id!)}/objects/${encodeURIComponent(file.id!)}`, { + status: 200, + contentDisposition: "attachment; filename*=UTF-8''opaque-storage-id.xlsx", + body: 'workbook bytes', + }); + + const job = makeJob([file]); + asInternals(job).submissionDir = tmpDir; + + const writtenName = await job.downloadAndWriteFile(file); + + expect(writtenName).toBe('Sample_-_Superstore.xlsx'); + expect(await fsp.readFile(path.join(tmpDir, 'Sample_-_Superstore.xlsx'), 'utf8')) + .toBe('workbook bytes'); + expect(await fsp.stat(path.join(tmpDir, 'opaque-storage-id.xlsx')).catch(() => null)).toBeNull(); + }); + it('resolves concurrent header destinations without provisional-name false conflicts', async () => { const renamed: TFile = { id: 'renamed-id', diff --git a/api/src/job-helpers.test.ts b/api/src/job-helpers.test.ts index 3509f9db..0c31f9d3 100644 --- a/api/src/job-helpers.test.ts +++ b/api/src/job-helpers.test.ts @@ -177,6 +177,33 @@ describe('resolveOriginalName', () => { ), ).toBe('nested/file.txt'); }); + + it('keeps the requested name when an old file server advertises the opaque object basename', () => { + expect( + resolveOriginalName( + responseWithHeader("attachment; filename*=UTF-8''storage-id.xlsx"), + { name: 'Sample_-_Superstore.xlsx', id: 'storage-id' }, + ), + ).toBe('Sample_-_Superstore.xlsx'); + }); + + it('keeps the requested name for a legacy opaque filename header', () => { + expect( + resolveOriginalName( + responseWithHeader('attachment; filename="storage-id.csv"'), + { name: 'original.csv', id: 'storage-id' }, + ), + ).toBe('original.csv'); + }); + + it('keeps an authoritative nested filename even when its basename matches the object id', () => { + expect( + resolveOriginalName( + responseWithHeader("attachment; filename*=UTF-8''exports%2Fstorage-id.csv"), + { name: 'original.csv', id: 'storage-id' }, + ), + ).toBe('exports/storage-id.csv'); + }); }); describe('mimeTypeFor', () => { diff --git a/api/src/job.ts b/api/src/job.ts index eceffdfd..e221ae19 100644 --- a/api/src/job.ts +++ b/api/src/job.ts @@ -179,11 +179,24 @@ export function resolveOriginalName(response: Response, file: TFile): string { const header = response.headers.get('content-disposition'); if (!header) return fallback; + const preferRequestedName = (candidate: string): string => { + /* Older file servers advertised path.basename(objectName) when an + * S3-compatible backend omitted original-filename user metadata. That + * basename is ``, so it is a storage identifier rather + * than an authoritative destination. Preserve the caller's requested name + * during rolling upgrades instead of exposing the opaque id in /mnt/data. */ + const opaqueStem = path.basename(candidate, path.extname(candidate)); + const isFlatObjectBasename = candidate === path.basename(candidate); + return file.name && file.id && isFlatObjectBasename && opaqueStem === file.id + ? file.name + : candidate; + }; + const star = header.match(/filename\*=(?:UTF-8'[^']*')?([^;]+)/i); if (star) { const raw = star[1].trim(); try { - return decodeURIComponent(raw); + return preferRequestedName(decodeURIComponent(raw)); } catch { /* Malformed percent-encoding (e.g. `%ZZ`) — fall through to the legacy * forms. The same header may emit both `filename*=` and a legacy @@ -194,7 +207,7 @@ export function resolveOriginalName(response: Response, file: TFile): string { const match = header.match(/filename="([^"]+)"/i) ?? header.match(/filename=([^\s;]+)/i); - return match ? match[1] : fallback; + return match ? preferRequestedName(match[1]) : fallback; } /** diff --git a/service/src/egress-gateway.test.ts b/service/src/egress-gateway.test.ts index 9f72da54..99303850 100644 --- a/service/src/egress-gateway.test.ts +++ b/service/src/egress-gateway.test.ts @@ -632,7 +632,10 @@ describe('egress gateway routes', () => { test('downloads scoped objects by unwrapping handles', async () => { upstreamResponse = new Response('file-body', { status: 200, - headers: { 'Content-Type': 'text/plain' }, + headers: { + 'Content-Type': 'text/plain', + 'Content-Disposition': "attachment; filename*=UTF-8''file_123.csv", + }, }); const readSession = sessionHandle({ dir: 'read', sessionId: 'sess_input' }); const object = objectHandle({}); @@ -643,10 +646,29 @@ describe('egress gateway routes', () => { expect(response.status).toBe(200); expect(await response.text()).toBe('file-body'); + expect(response.headers.get('content-disposition')).toBe('attachment'); expect(upstreamCalls[0].url).toBe('http://file-server/sessions/sess_input/objects/file_123'); expect(header(upstreamCalls[0].init, INTERNAL_SERVICE_TOKEN_HEADER)).toBe(INTERNAL_TOKEN); }); + test('preserves an authoritative upstream download filename', async () => { + upstreamResponse = new Response('file-body', { + status: 200, + headers: { + 'Content-Disposition': "attachment; filename*=UTF-8''reports%2Fdata.csv", + }, + }); + const readSession = sessionHandle({ dir: 'read', sessionId: 'sess_input' }); + const object = objectHandle({}); + + const response = await gatewayFetch(`/sessions/${readSession}/objects/${object}`, { + headers: grantHeader(), + }); + + expect(response.headers.get('content-disposition')) + .toBe("attachment; filename*=UTF-8''reports%2Fdata.csv"); + }); + test('downloads required dirkeep markers without allowing unrelated markers', async () => { upstreamResponse = new Response('marker-body', { status: 200, diff --git a/service/src/egress-gateway.ts b/service/src/egress-gateway.ts index 3499de80..d4d3e713 100644 --- a/service/src/egress-gateway.ts +++ b/service/src/egress-gateway.ts @@ -42,6 +42,7 @@ import { isValidId } from './utils'; import logger from './logger'; import { parseBoundedContentLength } from './http-limits'; import { validateEgressGatewayHardenedConfig } from './secure-startup'; +import { isOpaqueObjectContentDisposition } from './file-metadata'; export const app: Express = express(); app.disable('x-powered-by'); @@ -347,9 +348,13 @@ function responseHeaders(fetchResponse: globalThis.Response): Record = {}, +): void { res.status(fetchResponse.status); - res.set(responseHeaders(fetchResponse)); + res.set({ ...responseHeaders(fetchResponse), ...headerOverrides }); if (!fetchResponse.body) { res.end(); return; @@ -631,7 +636,13 @@ app.get('/sessions/:sessionHandle/objects/:objectHandle', async (req, res) => { ), { headers: injectTraceHeaders(internalServiceHeaders()) }, ); - return pipeFetchResponse(upstream, res); + const headerOverrides = isOpaqueObjectContentDisposition( + upstream.headers.get('content-disposition'), + object.id, + ) + ? { 'content-disposition': 'attachment' } + : {}; + return pipeFetchResponse(upstream, res, headerOverrides); } catch (error) { return sendEgressError(req, res, error); } diff --git a/service/src/file-metadata.test.ts b/service/src/file-metadata.test.ts new file mode 100644 index 00000000..082a4215 --- /dev/null +++ b/service/src/file-metadata.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'bun:test'; +import { + contentDispositionForOriginalFilename, + decodeOriginalFilename, + isOpaqueObjectContentDisposition, + originalFilenameFromMetadata, +} from './file-metadata'; + +describe('originalFilenameFromMetadata', () => { + it('returns undefined rather than treating an object-key basename as original metadata', () => { + expect(originalFilenameFromMetadata(undefined)).toBeUndefined(); + expect(originalFilenameFromMetadata({ 'content-type': 'application/octet-stream' })).toBeUndefined(); + }); + + it('decodes the base64 filename written by the file server', () => { + expect(originalFilenameFromMetadata({ + 'original-filename': Buffer.from('Sample_-_Superstore.xlsx').toString('base64'), + 'original-filename-encoded': 'base64', + })).toBe('Sample_-_Superstore.xlsx'); + }); + + it('supports legacy plain-text filename metadata', () => { + expect(originalFilenameFromMetadata({ + 'original-filename': 'report.csv', + })).toBe('report.csv'); + }); +}); + +describe('decodeOriginalFilename', () => { + it('retains object-key fallback behavior for listing responses', () => { + expect(decodeOriginalFilename(undefined, 'opaque-id.xlsx')).toBe('opaque-id.xlsx'); + }); +}); + +describe('contentDispositionForOriginalFilename', () => { + it('preserves attachment semantics when filename metadata is unavailable', () => { + expect(contentDispositionForOriginalFilename(undefined)).toBe('attachment'); + }); + + it('encodes a verified original filename', () => { + expect(contentDispositionForOriginalFilename('reports/收益.csv')) + .toBe("attachment; filename*=UTF-8''reports%2F%E6%94%B6%E7%9B%8A.csv"); + }); +}); + +describe('isOpaqueObjectContentDisposition', () => { + it('recognizes extended and legacy storage-id basenames', () => { + expect(isOpaqueObjectContentDisposition( + "attachment; filename*=UTF-8''raw-object-id.xlsx", + 'raw-object-id', + )).toBe(true); + expect(isOpaqueObjectContentDisposition( + 'attachment; filename="raw-object-id.csv"', + 'raw-object-id', + )).toBe(true); + }); + + it('does not replace verified or nested filenames', () => { + expect(isOpaqueObjectContentDisposition( + 'attachment; filename="report.csv"', + 'raw-object-id', + )).toBe(false); + expect(isOpaqueObjectContentDisposition( + "attachment; filename*=UTF-8''exports%2Fraw-object-id.csv", + 'raw-object-id', + )).toBe(false); + }); +}); diff --git a/service/src/file-metadata.ts b/service/src/file-metadata.ts new file mode 100644 index 00000000..13a45623 --- /dev/null +++ b/service/src/file-metadata.ts @@ -0,0 +1,62 @@ +import path from 'path'; + +/** + * Reads a verified original filename from S3 user metadata. Absence remains + * distinct from the object's opaque storage-key basename so callers can avoid + * advertising the latter as an authoritative filename. + */ +export function originalFilenameFromMetadata( + metadata: Record | undefined, +): string | undefined { + const encodedFilename = metadata?.['original-filename']; + if (!encodedFilename) return undefined; + + if (metadata?.['original-filename-encoded'] === 'base64') { + return Buffer.from(encodedFilename, 'base64').toString('utf8'); + } + + return encodedFilename; +} + +export function decodeOriginalFilename( + metadata: Record | undefined, + fallbackName: string, +): string { + return originalFilenameFromMetadata(metadata) ?? fallbackName; +} + +export function contentDispositionForOriginalFilename( + originalFilename: string | undefined, +): string { + if (!originalFilename) return 'attachment'; + return `attachment; filename*=UTF-8''${encodeURIComponent(originalFilename)}`; +} + +function filenameFromContentDisposition(contentDisposition: string | null): string | undefined { + if (!contentDisposition) return undefined; + const star = contentDisposition.match(/filename\*=(?:UTF-8'[^']*')?([^;]+)/i); + if (star) { + try { + return decodeURIComponent(star[1].trim()); + } catch { + // A valid legacy filename may still follow a malformed extended value. + } + } + const legacy = contentDisposition.match(/filename="([^"]+)"/i) + ?? contentDisposition.match(/filename=([^\s;]+)/i); + return legacy?.[1]; +} + +/** + * Detects the legacy file-server fallback ``. The egress + * gateway has the unsealed object id, so it can remove this unverified name + * before forwarding the response to a runner that only sees sealed handles. + */ +export function isOpaqueObjectContentDisposition( + contentDisposition: string | null, + objectId: string, +): boolean { + const candidate = filenameFromContentDisposition(contentDisposition); + if (!candidate || candidate !== path.basename(candidate)) return false; + return path.basename(candidate, path.extname(candidate)) === objectId; +} diff --git a/service/src/file-server.ts b/service/src/file-server.ts index 6d226c06..f9293e48 100644 --- a/service/src/file-server.ts +++ b/service/src/file-server.ts @@ -17,6 +17,11 @@ import { shutdownTelemetry, traceHttpRequest } from './telemetry'; import logger from './fileServerLogger'; import { env } from './config'; import { redisKeepAliveOptions } from './redis-options'; +import { + contentDispositionForOriginalFilename, + decodeOriginalFilename, + originalFilenameFromMetadata, +} from './file-metadata'; const { INSTANCE_ID } = env; @@ -458,11 +463,11 @@ app.get('/sessions/:session_id/objects/:objectId/metadata', async (req, res) => } const stat: Partial = await minioClient.statObject(bucketName, objectName); - const originalFilename = decodeOriginalFilename(stat.metaData, path.basename(objectName)); + const originalFilename = originalFilenameFromMetadata(stat.metaData); return res.status(200).json({ name: objectName, - originalFilename, + ...(originalFilename ? { originalFilename } : {}), size: stat.size, lastModified: stat.lastModified, etag: stat.etag, @@ -508,17 +513,7 @@ app.get('/sessions/:session_id/objects/:objectId', async (req, res) => { const stat: Partial = await minioClient.statObject(bucketName, objectName); - let originalFilename = path.basename(objectName); - if (stat.metaData?.['original-filename-encoded'] === 'base64' && stat.metaData['original-filename'] != null) { - try { - originalFilename = Buffer.from(stat.metaData['original-filename'], 'base64').toString('utf8'); - } catch (err) { - logger.warn('Failed to decode filename from metadata, using fallback', { error: err }); - originalFilename = stat.metaData['original-filename'] ?? path.basename(objectName); - } - } else if (stat.metaData?.['original-filename'] != null) { - originalFilename = stat.metaData['original-filename']; - } + const originalFilename = originalFilenameFromMetadata(stat.metaData); logger.info(`[${INSTANCE_ID}] File found: ${objectName}`); @@ -526,8 +521,11 @@ app.get('/sessions/:session_id/objects/:objectId', async (req, res) => { res.removeHeader('Transfer-Encoding'); res.removeHeader('Date'); - const encodedFilename = encodeURIComponent(originalFilename); - res.setHeader('Content-Disposition', `attachment; filename*=UTF-8''${encodedFilename}`); + /* An object-key basename is only a storage identifier, not an original + * filename. If an S3-compatible backend drops user metadata, retain + * attachment semantics but omit the filename so the runner uses its + * caller-supplied destination. */ + res.setHeader('Content-Disposition', contentDispositionForOriginalFilename(originalFilename)); if (stat.metaData?.['content-type'] != null) { res.setHeader('Content-Type', stat.metaData['content-type']); } @@ -573,27 +571,6 @@ app.get('/sessions/:session_id/objects/:objectId', async (req, res) => { } }); -/** - * Decodes the original filename from metadata. - * Handles both base64-encoded and plain text filenames for consistency. - */ -function decodeOriginalFilename(metadata: Record | undefined, fallbackName: string): string { - if (!metadata) return fallbackName; - - const encodedFilename = metadata['original-filename']; - const encodingType = metadata['original-filename-encoded']; - - if (encodedFilename && encodingType === 'base64') { - try { - return Buffer.from(encodedFilename, 'base64').toString('utf8'); - } catch { - return encodedFilename; - } - } - - return encodedFilename || fallbackName; -} - /** * Extracts session_id and file_id from object name (format: {session_id}/{file_id}.ext) */ From a1fd45eedc68c666aa689fa40f3e896b95c58f02 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 10:53:00 -0400 Subject: [PATCH 5/8] =?UTF-8?q?=F0=9F=AA=AB=20chore:=20Detect=20Compose=20?= =?UTF-8?q?Sandbox=20Clock=20Drift=20(#81)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guest clock drift past the 30s execution-manifest tolerance makes every /v1/exec fail with "not_yet_valid" while both health endpoints keep reporting healthy, so the stack looks fine while nothing runs (#37). The healthcheck already detects this, but it stays disabled unless an orchestrator opts in, and the Compose files never did -- only the Helm chart set it. Opt in there too, at the same 10s the chart uses. The 2s probe timeout keeps the check inside both files' healthcheck timeouts (3s and 5s) and leaves headroom under the 30s tolerance. --- docker-compose.local-dev.yml | 5 +++++ docker-compose.yaml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/docker-compose.local-dev.yml b/docker-compose.local-dev.yml index bf58b596..4d8f5a33 100644 --- a/docker-compose.local-dev.yml +++ b/docker-compose.local-dev.yml @@ -42,6 +42,11 @@ services: - CODEAPI_INTERNAL_SERVICE_TOKEN=${CODEAPI_INTERNAL_SERVICE_TOKEN:-localdev-internal-service-token} - SANDBOX_ALLOWED_LOCAL_NETWORK_PORT=3033 - SANDBOX_FORWARD_TARGET=tool_call_server:3033 + # Guest clock drift silently fails every exec with "not_yet_valid" + # once it passes the 30s execution-manifest tolerance (#37). Opt in so + # the healthcheck reports it; set the limit to 0 to disable. + - SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS=${SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS:-10} + - SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS=${SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS:-2} healthcheck: test: ["CMD", "/usr/local/bin/sandbox-runner-healthcheck.sh"] interval: 10s diff --git a/docker-compose.yaml b/docker-compose.yaml index ac98d917..00cad657 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -161,6 +161,11 @@ services: - SANDBOX_FORWARD_TARGET=egress_gateway:3190 - SANDBOX_REQUIRE_EGRESS_MANIFEST=${SANDBOX_REQUIRE_EGRESS_MANIFEST:-true} - SANDBOX_EXECUTION_MANIFEST_PUBLIC_KEY=${SANDBOX_EXECUTION_MANIFEST_PUBLIC_KEY:-MCowBQYDK2VwAyEAeY3PRoTS3adfU6E3gQUB5hSZdrdMSw6OrKkH4UhYh0U=} + # Guest clock drift silently fails every exec with "not_yet_valid" + # once it passes the 30s execution-manifest tolerance (#37). Opt in + # so the healthcheck reports it; set the limit to 0 to disable. + - SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS=${SANDBOX_RUNNER_CLOCK_SKEW_LIVENESS_LIMIT_SECONDS:-10} + - SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS=${SANDBOX_RUNNER_HEALTHCHECK_TIMEOUT_SECONDS:-2} depends_on: egress_gateway: condition: service_healthy From 0eb0f3a30e984f23fa98d0cc48acc8abd21cb184 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 10:53:13 -0400 Subject: [PATCH 6/8] =?UTF-8?q?=F0=9F=9B=B3=20fix:=20Fetch=20Bitnami=20Sub?= =?UTF-8?q?charts=20From=20OCI=20(#83)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(helm): resolve Bitnami subcharts over OCI Bitnami distributes charts OCI-only. The classic charts.bitnami.com index still lists redis 24.1.0 and minio 17.0.21, but resolves them to an oci:// download URL that HTTP-repository getters cannot follow, so FluxCD's source-controller fails dependency resolution outright with 'unsupported protocol scheme "oci"' (#21). Point both dependencies at the OCI registry directly. Requires Helm >= 3.8. Reported by @meroo36. * docs(helm): require Helm >= 3.8 for OCI subchart resolution Addresses codex review on #83. The README's "Helm 3.x" prerequisite and setup-local.sh's existence-only check both allowed 3.0-3.7, where OCI dependency references are not resolved without an experimental flag -- so the documented setup flow would fail at dependency resolution rather than with a clear message. State the real minimum, and reject older Helm in setup-local.sh before it gets that far. Also drop the classic bitnami repo registration, which the OCI references no longer use; verified 'helm dependency update' resolves both subcharts with that repo removed from the local Helm config. * chore(helm): bump chart to 0.3.1 for the dependency source change Addresses codex review on #83. Changing where the subcharts resolve from is a chart-level change, and Chart.yaml's own version comment asks for a bump. Leaving 0.3.0 in place lets consumers reconciling on chart version treat the corrected chart as the already-seen 0.3.0 artifact and keep the broken HTTP dependency metadata. Matches 4b72e9d, which bumped the chart for the same reason. --- helm/codeapi/Chart.yaml | 11 ++++++++--- helm/codeapi/README.md | 4 +++- helm/setup-local.sh | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/helm/codeapi/Chart.yaml b/helm/codeapi/Chart.yaml index 9737e908..4e122bbf 100644 --- a/helm/codeapi/Chart.yaml +++ b/helm/codeapi/Chart.yaml @@ -3,7 +3,7 @@ apiVersion: v2 name: codeapi description: A Helm chart for Code Interpreter API - scalable code execution service type: application -version: 0.3.0 # Chart version (bump this when you change the chart) +version: 0.3.1 # Chart version (bump this when you change the chart) appVersion: "2.0.0" # App version (bump this when you change the app) # Keywords for searching @@ -18,12 +18,17 @@ maintainers: url: https://github.com/danny-avila # Dependencies (we'll use these for Redis and MinIO) +# Bitnami distributes charts OCI-only. The classic charts.bitnami.com/bitnami +# index still lists these versions but resolves them to an oci:// download URL, +# which HTTP-repository getters (notably FluxCD's source-controller) cannot +# follow -- dependency resolution fails outright. Point at the OCI registry +# directly; requires Helm >= 3.8. dependencies: - name: redis version: "24.1.0" - repository: "https://charts.bitnami.com/bitnami" + repository: "oci://registry-1.docker.io/bitnamicharts" condition: redis.enabled - name: minio version: "17.0.21" - repository: "https://charts.bitnami.com/bitnami" + repository: "oci://registry-1.docker.io/bitnamicharts" condition: minio.enabled diff --git a/helm/codeapi/README.md b/helm/codeapi/README.md index fcdef01d..29dc894a 100644 --- a/helm/codeapi/README.md +++ b/helm/codeapi/README.md @@ -6,7 +6,9 @@ Deploy the horizontally-scalable Code Interpreter API stack to Kubernetes. - Docker Desktop with Kubernetes enabled, OR - Minikube installed (`brew install minikube` / `choco install minikube`) -- Helm 3.x (`brew install helm` / `choco install kubernetes-helm`) +- Helm >= 3.8 (`brew install helm` / `choco install kubernetes-helm`) — the + redis and minio subcharts are pulled from an OCI registry, which older Helm + releases only support behind an experimental flag - kubectl (`brew install kubectl` / `choco install kubernetes-cli`) ## Execution manifest signing keys (required) diff --git a/helm/setup-local.sh b/helm/setup-local.sh index f71c39bb..f51889d6 100755 --- a/helm/setup-local.sh +++ b/helm/setup-local.sh @@ -32,9 +32,38 @@ check_command() { echo "✓ $1 found" } +# The chart's subchart dependencies are OCI references, which Helm only +# resolves without an experimental flag from 3.8 onward. +check_helm_version() { + local required_major=3 required_minor=8 + local raw major minor + raw=$(helm version --template '{{.Version}}' 2>/dev/null || true) + if [ -z "$raw" ]; then + echo "❌ could not determine the installed Helm version (need >= ${required_major}.${required_minor})." + exit 1 + fi + raw=${raw#v} + major=${raw%%.*} + minor=${raw#*.} + minor=${minor%%.*} + case "$major$minor" in + *[!0-9]*|'') + echo "❌ could not parse the installed Helm version '$raw' (need >= ${required_major}.${required_minor})." + exit 1 + ;; + esac + if [ "$major" -lt "$required_major" ] || + { [ "$major" -eq "$required_major" ] && [ "$minor" -lt "$required_minor" ]; }; then + echo "❌ Helm $raw is too old. The chart's OCI subchart dependencies need >= ${required_major}.${required_minor}." + exit 1 + fi + echo "✓ helm $raw supports OCI dependencies" +} + echo "📋 Checking prerequisites..." check_command docker check_command helm +check_helm_version check_command kubectl check_command "$CLUSTER_TYPE" echo "" @@ -96,8 +125,8 @@ fi # Add Helm repos and update dependencies echo "📚 Setting up Helm dependencies..." -helm repo add bitnami https://charts.bitnami.com/bitnami 2>/dev/null || true -helm repo update +# Subcharts resolve from oci://registry-1.docker.io/bitnamicharts, so no +# classic chart repository needs registering. helm dependency update ./helm/codeapi echo "" From 117c23e76405eee153ec9f07b2a6bc60f9a1ad94 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 12:42:04 -0400 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=9B=9C=20feat:=20Add=20Networkless=20?= =?UTF-8?q?BYOM=20File=20Relay=20(#80)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add networkless BYOM file relay * fix: harden managed file relay * fix: fence relay lifecycle races * fix: order relay handoffs by registration * fix: gate relay workers on runtime readiness * fix: preserve legacy marker listings * fix: recover reclaimed relay staging --- api/src/config.ts | 1 + api/src/download.test.ts | 8 + api/src/job-cleanup.test.ts | 87 ++++ api/src/job.ts | 60 ++- packages/code/README.md | 46 ++- packages/code/src/cli.test.ts | 75 ++++ packages/code/src/cli.ts | 297 ++++++++++---- packages/code/src/protocol.ts | 7 +- packages/code/src/relay-runtime.test.ts | 506 ++++++++++++++++++++++++ packages/code/src/relay-runtime.ts | 425 ++++++++++++++++++++ packages/code/src/relay.test.ts | 383 ++++++++++++++++++ packages/code/src/relay.ts | 276 +++++++++++++ packages/code/src/runtime.test.ts | 39 ++ packages/code/src/runtime.ts | 12 +- packages/code/src/worker.test.ts | 104 +++++ packages/code/src/worker.ts | 36 ++ service/src/bridge/pairing.ts | 5 +- service/src/bridge/router.test.ts | 1 + service/src/bridge/router.ts | 58 ++- service/src/bridge/store.test.ts | 156 +++++++- service/src/bridge/store.ts | 163 +++++++- 21 files changed, 2625 insertions(+), 120 deletions(-) create mode 100644 packages/code/src/relay-runtime.test.ts create mode 100644 packages/code/src/relay-runtime.ts create mode 100644 packages/code/src/relay.test.ts create mode 100644 packages/code/src/relay.ts diff --git a/api/src/config.ts b/api/src/config.ts index 443745f9..d824c0db 100644 --- a/api/src/config.ts +++ b/api/src/config.ts @@ -103,6 +103,7 @@ export const config = { max_input_files: safeInt(process.env.SANDBOX_MAX_INPUT_FILES, 256), prime_concurrency: safeInt(process.env.SANDBOX_PRIME_CONCURRENCY, 8), egress_gateway_url: egressGatewayUrl, + file_relay_token: process.env.SANDBOX_FILE_RELAY_TOKEN ?? '', file_server_url: process.env.FILE_SERVER_URL ?? '', max_nesting_depth: safeInt(process.env.SANDBOX_MAX_NESTING_DEPTH, 10), max_path_length: safeInt(process.env.SANDBOX_MAX_PATH_LENGTH, 256), diff --git a/api/src/download.test.ts b/api/src/download.test.ts index 76e21395..5372a40c 100644 --- a/api/src/download.test.ts +++ b/api/src/download.test.ts @@ -110,11 +110,13 @@ let serverPort = 0; const routes = new Map(); let originalFileServerUrl: string; let originalEgressGatewayUrl: string; +let originalFileRelayToken: string; let originalPerJobUids: boolean; beforeAll(() => { originalFileServerUrl = config.file_server_url; originalEgressGatewayUrl = config.egress_gateway_url; + originalFileRelayToken = config.file_relay_token; originalPerJobUids = config.per_job_uids; server = Bun.serve({ port: 0, @@ -151,6 +153,7 @@ beforeAll(() => { afterAll(() => { (config as { file_server_url: string }).file_server_url = originalFileServerUrl; (config as { egress_gateway_url: string }).egress_gateway_url = originalEgressGatewayUrl; + (config as { file_relay_token: string }).file_relay_token = originalFileRelayToken; (config as { per_job_uids: boolean }).per_job_uids = originalPerJobUids; server.stop(true); }); @@ -164,6 +167,7 @@ beforeEach(async () => { afterEach(async () => { (config as { egress_gateway_url: string }).egress_gateway_url = originalEgressGatewayUrl; + (config as { file_relay_token: string }).file_relay_token = originalFileRelayToken; (config as { file_server_url: string }).file_server_url = `http://127.0.0.1:${serverPort}`; (config as { per_job_uids: boolean }).per_job_uids = false; await fsp.rm(tmpDir, { recursive: true, force: true }); @@ -208,6 +212,7 @@ describe('downloadAndWriteFile / RFC 5987 round-trip', () => { name: 'gateway-fallback.txt', }; let sawGrantHeader = false; + let sawRelayToken = false; let sawInternalHeader = false; routes.set(`/sessions/${encodeURIComponent(file.storage_session_id!)}/objects/${encodeURIComponent(file.id!)}`, { status: 200, @@ -215,10 +220,12 @@ describe('downloadAndWriteFile / RFC 5987 round-trip', () => { body: 'gateway bytes', onRequest(req) { sawGrantHeader = req.headers.get('x-codeapi-egress-grant') === 'opaque-grant'; + sawRelayToken = req.headers.get('x-librechat-code-relay-token') === 'relay-secret'; sawInternalHeader = req.headers.has('x-codeapi-internal-token'); }, }); (config as { egress_gateway_url: string }).egress_gateway_url = `http://127.0.0.1:${serverPort}`; + (config as { file_relay_token: string }).file_relay_token = 'relay-secret'; (config as { file_server_url: string }).file_server_url = 'http://127.0.0.1:1'; const job = new Job({ @@ -238,6 +245,7 @@ describe('downloadAndWriteFile / RFC 5987 round-trip', () => { expect(writtenName).toBe('gateway.txt'); expect(sawGrantHeader).toBe(true); + expect(sawRelayToken).toBe(true); expect(sawInternalHeader).toBe(false); expect(await fsp.readFile(path.join(tmpDir, 'gateway.txt'), 'utf8')).toBe('gateway bytes'); }); diff --git a/api/src/job-cleanup.test.ts b/api/src/job-cleanup.test.ts index de4b52e5..0b45a8ab 100644 --- a/api/src/job-cleanup.test.ts +++ b/api/src/job-cleanup.test.ts @@ -22,6 +22,10 @@ interface CleanupInternals { jobIdentity?: SandboxJobIdentity; } +interface MarkerInternals { + autoLoadDirkeep(): Promise; +} + function makeRuntime(): Runtime { return { language: 'bash', @@ -196,4 +200,87 @@ describe('Job cleanup', () => { await fsp.rm(workspace, { recursive: true, force: true }); } }); + + test('bounds inherited marker listing concurrency', async () => { + const files: TFile[] = Array.from( + { length: config.prime_concurrency + 4 }, + (_, index) => ({ + id: `input-${index}`, + name: `input-${index}.txt`, + storage_session_id: `storage-${index}`, + }), + ); + const job = new Job({ + session_id: 'marker-concurrency', + runtime: makeRuntime(), + files, + args: [], + stdin: '', + timeouts: { compile: 5000, run: 5000 }, + cpu_times: { compile: 5000, run: 5000 }, + memory_limits: { compile: 100_000_000, run: 100_000_000 }, + }); + const originalFetch = globalThis.fetch; + let active = 0; + let maxActive = 0; + globalThis.fetch = async () => { + active += 1; + maxActive = Math.max(maxActive, active); + await new Promise(resolve => setTimeout(resolve, 10)); + active -= 1; + return Response.json([]); + }; + + try { + await (job as unknown as MarkerInternals).autoLoadDirkeep(); + expect(maxActive).toBeLessThanOrEqual(config.prime_concurrency); + expect(maxActive).toBeGreaterThan(1); + } finally { + globalThis.fetch = originalFetch; + } + }); + + test('retries relay backpressure and rejects persistent marker-list failures', async () => { + const makeMarkerJob = (): Job => + new Job({ + session_id: 'marker-backpressure', + runtime: makeRuntime(), + files: [ + { + id: 'input-1', + name: 'input.txt', + storage_session_id: 'storage-1', + }, + ], + args: [], + stdin: '', + timeouts: { compile: 5000, run: 5000 }, + cpu_times: { compile: 5000, run: 5000 }, + memory_limits: { compile: 100_000_000, run: 100_000_000 }, + }); + const originalFetch = globalThis.fetch; + let attempts = 0; + globalThis.fetch = async () => { + attempts += 1; + if (attempts === 1) { + return new Response(null, { + status: 503, + headers: { 'Retry-After': '0' }, + }); + } + return Response.json([]); + }; + + try { + await (makeMarkerJob() as unknown as MarkerInternals).autoLoadDirkeep(); + expect(attempts).toBe(2); + + globalThis.fetch = async () => new Response(null, { status: 502 }); + await expect( + (makeMarkerJob() as unknown as MarkerInternals).autoLoadDirkeep(), + ).rejects.toThrow('HTTP error loading .dirkeep markers: 502'); + } finally { + globalThis.fetch = originalFetch; + } + }); }); diff --git a/api/src/job.ts b/api/src/job.ts index e221ae19..1e78ae75 100644 --- a/api/src/job.ts +++ b/api/src/job.ts @@ -57,6 +57,7 @@ export { } from './validation'; const AUTO_LOAD_DIRKEEP_TIMEOUT_MS = 10000; +const AUTO_LOAD_DIRKEEP_RETRIES = 2; /** * Bridges a `fetch` response body to a Node-stream Readable. The types at the @@ -1131,6 +1132,9 @@ export class Job { return injectTraceHeaders({ ...headers, [EGRESS_GRANT_HEADER]: this.egressGrantToken, + ...(config.file_relay_token + ? { 'X-LibreChat-Code-Relay-Token': config.file_relay_token } + : {}), }); } @@ -1143,8 +1147,11 @@ export class Job { .filter(f => !isDirkeep(f.name)) .map(f => f.name); - const fetches = Array.from(sessionIds).map(sid => this.fetchSessionMarkers(sid)); - const results = await Promise.all(fetches); + const results = await mapWithConcurrency( + Array.from(sessionIds), + config.prime_concurrency, + sid => this.fetchSessionMarkers(sid), + ); let added = 0; let hitCap = false; @@ -1166,7 +1173,7 @@ export class Job { /** * Fetches normalized objects for one inherited session and returns the * `.dirkeep` markers belonging to exactly that session. Guards against: - * - non-OK responses (empty list, no throw) + * - legacy 404 responses (no marker support) and transient backpressure * - non-array JSON bodies * - missing/malformed id/name/storage_session_id fields * - MinIO prefix-list leakage (`abc` prefix also matches `abcdef/...`) @@ -1178,20 +1185,43 @@ export class Job { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), AUTO_LOAD_DIRKEEP_TIMEOUT_MS); try { - const res = await fetch( - `${this.fileEgressBaseUrl()}/sessions/${encodeURIComponent(sid)}/objects?detail=normalized`, - { - headers: this.fileEgressHeaders(), - signal: controller.signal, - }, - ); - if (!res.ok) return []; - const data: unknown = await res.json(); - if (!Array.isArray(data)) return []; - return data.filter(isNormalizedObjectForSession(sid)); + for (let attempt = 0; attempt <= AUTO_LOAD_DIRKEEP_RETRIES; attempt += 1) { + const res = await fetch( + `${this.fileEgressBaseUrl()}/sessions/${encodeURIComponent(sid)}/objects?detail=normalized`, + { + headers: this.fileEgressHeaders(), + signal: controller.signal, + }, + ); + if (res.status === 503 && attempt < AUTO_LOAD_DIRKEEP_RETRIES) { + await res.body?.cancel().catch(() => {}); + const retryAfterSeconds = Number(res.headers.get('retry-after')); + await sleep( + Number.isFinite(retryAfterSeconds) + ? Math.min(1000, Math.max(25, retryAfterSeconds * 1000)) + : 100, + controller.signal, + ); + continue; + } + if (res.status === 404) { + await res.body?.cancel().catch(() => {}); + return []; + } + if (!res.ok) { + await res.body?.cancel().catch(() => {}); + throw new Error(`HTTP error loading .dirkeep markers: ${res.status}`); + } + const data: unknown = await res.json(); + if (!Array.isArray(data)) { + throw new Error('Invalid .dirkeep marker response'); + } + return data.filter(isNormalizedObjectForSession(sid)); + } + throw new Error('Exhausted .dirkeep marker retries'); } catch (err) { this.log.warn({ sessionId: sid, err }, 'Failed to auto-load .dirkeep markers'); - return []; + throw err; } finally { clearTimeout(timeout); } diff --git a/packages/code/README.md b/packages/code/README.md index 8e6c63b4..caf129b5 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -96,10 +96,48 @@ reports state loss instead of restarting it. The next assignment starts a new environment. Treat profile changes and Docker restarts as environment resets and preserve any needed workspace contents first. -This first local profile supports inline request files. By-reference inputs and -generated-file uploads require a worker-mediated file relay and are not yet -supported; the runtime remains networkless rather than opening general egress -to reach a file server. +By-reference inputs and generated-file uploads remain disabled unless the +worker-managed file relay is configured. Build the worker image, then point the +relay at the deployment's public egress-gateway base URL: + +```bash +docker build -t librechat-code-worker:local packages/code + +LIBRECHAT_CODE_FILE_RELAY_IMAGE=librechat-code-worker:local \ +LIBRECHAT_CODE_FILE_RELAY_UPSTREAM=https://code.example.com/egress \ +LIBRECHAT_CODE_EXECUTION_MANIFEST_PUBLIC_KEY='' \ +librechat-code run +``` + +The URL is illustrative; it must be the externally reachable HTTPS base URL +for the same Code API deployment's egress-gateway routes. Plain HTTP is accepted +only for loopback and Docker Desktop development hosts. Enabling the relay also +requires signed execution manifests. The worker creates a labeled internal +Docker network for each worker identity, connects the runtime only to that +network, and starts a separate hardened relay container on a labeled, +worker-specific egress network. Reused networks are accepted only when their +internal flag and ownership labels match the required profile. The relay +publishes no host port, accepts only the file-object read, normalized list, and +generated-object write routes, requires both its worker-derived token and the +assignment's scoped egress grant, refuses redirects, and caps request headers, +transfer size, duration, and concurrency. Its upstream is fixed at startup. +Overlapping worker incarnations use separate relay containers; the newly +registered incarnation removes stale relays only after Code API fences the old +incarnation, and orderly shutdown removes its own relay. Relay-capable workers +remain unavailable for dispatch until they activate and health-check the relay, +then confirm readiness for the exact registration incarnation and generation. +Each registration heartbeat revalidates the relay before renewing its +shorter-lived readiness confirmation, so a stopped relay ages out without +creating an availability gap during healthy heartbeats. +Stopped staging containers are reclaimed on the next activation; running +staging containers are reclaimed only after a conservative grace period. + +The trusted runner API can use this relay for file staging. User code still +runs in NsJail's separate network namespace with no interfaces, so it cannot +reach the relay or the public internet. Anyone with access to the Docker daemon +remains inside the trusted worker boundary and can inspect container +configuration and secrets. + Direct NsJail shares the Docker Desktop VM kernel and is suitable for local or operator-trusted development. Use a separate VM or MicroVM boundary for internet-facing execution of code from untrusted users. diff --git a/packages/code/src/cli.test.ts b/packages/code/src/cli.test.ts index 82701e42..00ae78c8 100644 --- a/packages/code/src/cli.test.ts +++ b/packages/code/src/cli.test.ts @@ -163,3 +163,78 @@ test('CLI reset does not require Docker runtime launch inputs', () => { /LIBRECHAT_CODE_(?:RUNTIME_IMAGE|DOCKER_SECCOMP_PROFILE|DOCKER_PACKAGES_PATH) is required/, ); }); + +test('CLI relay requires a fixed upstream URL', () => { + const result = spawnSync( + process.execPath, + [fileURLToPath(new URL('./cli.js', import.meta.url)), 'relay'], + { + encoding: 'utf8', + env: { + ...process.env, + LIBRECHAT_CODE_FILE_RELAY_UPSTREAM: undefined, + LIBRECHAT_CODE_FILE_RELAY_TOKEN: 'relay-secret', + }, + }, + ); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /LIBRECHAT_CODE_FILE_RELAY_UPSTREAM is required/); +}); + +test('CLI requires manifest verification before enabling the file relay', () => { + const result = spawnSync( + process.execPath, + [fileURLToPath(new URL('./cli.js', import.meta.url))], + { + encoding: 'utf8', + env: { + ...process.env, + LIBRECHAT_CODE_URL: 'https://code.example/v1', + LIBRECHAT_CODE_WORKER_TOKEN: 'worker-secret', + LIBRECHAT_CODE_WORKER_ID: 'engineering-vm', + LIBRECHAT_CODE_RUNTIME_SUPERVISOR: 'docker-macos-nsjail', + LIBRECHAT_CODE_RUNTIME_IMAGE: 'example/runtime:latest', + LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE: '../../seccomp/nsjail.json', + LIBRECHAT_CODE_DOCKER_PACKAGES_PATH: '.', + LIBRECHAT_CODE_FILE_RELAY_UPSTREAM: 'https://code.example/egress', + LIBRECHAT_CODE_EXECUTION_MANIFEST_PUBLIC_KEY: undefined, + }, + }, + ); + + assert.notEqual(result.status, 0); + assert.match( + result.stderr, + /LIBRECHAT_CODE_EXECUTION_MANIFEST_PUBLIC_KEY is required/, + ); +}); + +test('CLI treats a whitespace-only file relay upstream as disabled', () => { + const result = spawnSync( + process.execPath, + [fileURLToPath(new URL('./cli.js', import.meta.url))], + { + encoding: 'utf8', + timeout: 500, + env: { + ...process.env, + LIBRECHAT_CODE_URL: 'http://127.0.0.1:1/v1', + LIBRECHAT_CODE_WORKER_TOKEN: 'worker-secret', + LIBRECHAT_CODE_WORKER_ID: 'engineering-vm', + LIBRECHAT_CODE_RUNTIME_SUPERVISOR: 'docker-macos-nsjail', + LIBRECHAT_CODE_RUNTIME_IMAGE: 'example/runtime:latest', + LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE: '../../seccomp/nsjail.json', + LIBRECHAT_CODE_DOCKER_PACKAGES_PATH: '.', + LIBRECHAT_CODE_FILE_RELAY_UPSTREAM: ' ', + LIBRECHAT_CODE_EXECUTION_MANIFEST_PUBLIC_KEY: undefined, + LIBRECHAT_CODE_FILE_RELAY_IMAGE: undefined, + }, + }, + ); + + assert.doesNotMatch( + result.stderr, + /LIBRECHAT_CODE_(?:EXECUTION_MANIFEST_PUBLIC_KEY|FILE_RELAY_IMAGE) is required/, + ); +}); diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index 457af16f..9aa6f589 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -1,9 +1,11 @@ #!/usr/bin/env node -import { createHash } from 'node:crypto'; +import { createHash, createHmac, randomBytes } from 'node:crypto'; import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import { pairBridgeWorker } from './pairing.js'; +import { startFileRelay } from './relay.js'; +import { DockerFileRelaySupervisor } from './relay-runtime.js'; import { defaultBridgeIdentityPath, loadBridgeIdentity, @@ -31,6 +33,15 @@ function list(value: string | undefined): string[] { ); } +function positiveInteger(name: string, value: string | undefined, fallback: number): number { + if (value == null || value.trim().length === 0) return fallback; + const parsed = Number(value); + if (!Number.isSafeInteger(parsed) || parsed <= 0) { + throw new Error(`${name} must be a positive integer`); + } + return parsed; +} + const MACOS_NSJAIL_CAPABILITIES = [ 'SYS_ADMIN', 'SYS_CHROOT', @@ -71,6 +82,41 @@ async function pair(args: string[]): Promise { `Paired worker ${workerId}. Identity saved to ${identityPath}\n`, ); } + +async function relay(): Promise { + const handle = await startFileRelay({ + host: process.env.LIBRECHAT_CODE_FILE_RELAY_HOST?.trim() || '0.0.0.0', + port: positiveInteger( + 'LIBRECHAT_CODE_FILE_RELAY_PORT', + process.env.LIBRECHAT_CODE_FILE_RELAY_PORT, + 3000, + ), + upstreamUrl: required('LIBRECHAT_CODE_FILE_RELAY_UPSTREAM'), + token: required('LIBRECHAT_CODE_FILE_RELAY_TOKEN'), + maxBytes: positiveInteger( + 'LIBRECHAT_CODE_FILE_RELAY_MAX_BYTES', + process.env.LIBRECHAT_CODE_FILE_RELAY_MAX_BYTES, + 16 * 1024 * 1024, + ), + timeoutMs: positiveInteger( + 'LIBRECHAT_CODE_FILE_RELAY_TIMEOUT_MS', + process.env.LIBRECHAT_CODE_FILE_RELAY_TIMEOUT_MS, + 30_000, + ), + maxConcurrentRequests: positiveInteger( + 'LIBRECHAT_CODE_FILE_RELAY_MAX_CONCURRENT_REQUESTS', + process.env.LIBRECHAT_CODE_FILE_RELAY_MAX_CONCURRENT_REQUESTS, + 8, + ), + }); + process.stdout.write(`librechat-code: file relay listening at ${handle.url}\n`); + await new Promise((resolve) => { + process.once('SIGINT', resolve); + process.once('SIGTERM', resolve); + }); + await handle.close(); +} + async function run(runtimeSessionId?: string): Promise { const configuredWorkerId = process.env.LIBRECHAT_CODE_WORKER_ID?.trim(); const configuredIdentityPath = process.env.LIBRECHAT_CODE_IDENTITY_FILE?.trim(); @@ -135,6 +181,12 @@ async function run(runtimeSessionId?: string): Promise { expiresAt: pairedIdentity.expiresAt, } : undefined; + const fileRelayUpstream = + process.env.LIBRECHAT_CODE_FILE_RELAY_UPSTREAM?.trim(); + const fileRelayEnabled = + runtimeMode === 'docker-macos-nsjail' && + runtimeSessionId == null && + (fileRelayUpstream?.length ?? 0) > 0; const capabilities = { statefulWorkspace, sandboxProfile: @@ -142,6 +194,7 @@ async function run(runtimeSessionId?: string): Promise { (runtimeMode.startsWith('docker') ? 'oci-docker' : 'nsjail'), runtimes: list(process.env.LIBRECHAT_CODE_RUNTIMES), policyDigest: createHash('sha256').update(policy).digest('hex'), + ...(fileRelayEnabled ? { requiresReadyConfirmation: true } : {}), }; if (!isValidBridgeWorkerCapabilities(capabilities)) { throw new Error( @@ -151,84 +204,186 @@ async function run(runtimeSessionId?: string): Promise { const controller = new AbortController(); process.once('SIGINT', () => controller.abort()); process.once('SIGTERM', () => controller.abort()); - const worker = new BridgeWorker({ - codeApiUrl, - token: configuredToken, - identity: workerIdentity, - workerId, - runtimeSupervisor: - runtimeMode !== 'endpoint' - ? new DockerRuntimeSupervisor({ - image: - runtimeSessionId == null - ? required('LIBRECHAT_CODE_RUNTIME_IMAGE') - : process.env.LIBRECHAT_CODE_RUNTIME_IMAGE?.trim(), - ...(runtimeMode === 'docker-macos-nsjail' && runtimeSessionId == null - ? (() => { - const seccompProfile = resolve( - required('LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE'), - ); - const packagesPath = resolve( - required('LIBRECHAT_CODE_DOCKER_PACKAGES_PATH'), - ); - return { - capabilities: MACOS_NSJAIL_CAPABILITIES, - securityOptions: [`seccomp=${seccompProfile}`], - profileRevision: createHash('sha256') - .update(readFileSync(seccompProfile)) - .digest('hex'), - restartStoppedContainers: false, - bindMounts: [ - { - source: packagesPath, - target: '/pkgs', - readOnly: true, + const incarnationId = randomBytes(18).toString('base64url'); + const runtimeImage = + runtimeMode !== 'endpoint' + ? runtimeSessionId == null + ? required('LIBRECHAT_CODE_RUNTIME_IMAGE') + : process.env.LIBRECHAT_CODE_RUNTIME_IMAGE?.trim() + : undefined; + const macLaunchProfile = + runtimeMode === 'docker-macos-nsjail' && runtimeSessionId == null + ? (() => { + const seccompProfile = resolve( + required('LIBRECHAT_CODE_DOCKER_SECCOMP_PROFILE'), + ); + const packagesPath = resolve( + required('LIBRECHAT_CODE_DOCKER_PACKAGES_PATH'), + ); + return { + seccompProfile, + packagesPath, + profileRevision: createHash('sha256') + .update(readFileSync(seccompProfile)) + .digest('hex'), + }; + })() + : undefined; + const executionManifestPublicKey = fileRelayEnabled + ? required('LIBRECHAT_CODE_EXECUTION_MANIFEST_PUBLIC_KEY') + : undefined; + const fileRelayLimits = fileRelayEnabled + ? { + maxBytes: positiveInteger( + 'LIBRECHAT_CODE_FILE_RELAY_MAX_BYTES', + process.env.LIBRECHAT_CODE_FILE_RELAY_MAX_BYTES, + 16 * 1024 * 1024, + ), + timeoutMs: positiveInteger( + 'LIBRECHAT_CODE_FILE_RELAY_TIMEOUT_MS', + process.env.LIBRECHAT_CODE_FILE_RELAY_TIMEOUT_MS, + 30_000, + ), + maxConcurrentRequests: positiveInteger( + 'LIBRECHAT_CODE_FILE_RELAY_MAX_CONCURRENT_REQUESTS', + process.env.LIBRECHAT_CODE_FILE_RELAY_MAX_CONCURRENT_REQUESTS, + 8, + ), + } + : undefined; + const fileRelaySupervisor = + fileRelayEnabled && fileRelayUpstream + ? new DockerFileRelaySupervisor({ + workerId, + incarnationId, + image: required('LIBRECHAT_CODE_FILE_RELAY_IMAGE'), + upstreamUrl: fileRelayUpstream, + ...fileRelayLimits, + token: createHmac( + 'sha256', + pairedIdentity?.privateKey ?? + required('LIBRECHAT_CODE_WORKER_TOKEN', configuredToken), + ) + .update('librechat-code-file-relay-v1') + .digest('hex'), + }) + : undefined; + const fileRelayProfile = await fileRelaySupervisor?.prepare( + controller.signal, + ); + try { + const worker = new BridgeWorker({ + codeApiUrl, + token: configuredToken, + identity: workerIdentity, + workerId, + incarnationId, + runtimeSupervisor: + runtimeMode !== 'endpoint' + ? new DockerRuntimeSupervisor({ + image: runtimeImage, + ...(runtimeMode === 'docker-macos-nsjail' && runtimeSessionId == null + ? (() => { + const { seccompProfile, packagesPath, profileRevision } = + macLaunchProfile!; + return { + capabilities: MACOS_NSJAIL_CAPABILITIES, + securityOptions: [`seccomp=${seccompProfile}`], + profileRevision, + restartStoppedContainers: false, + ...(fileRelayProfile + ? { network: fileRelayProfile.network } + : {}), + bindMounts: [ + { + source: packagesPath, + target: '/pkgs', + readOnly: true, + }, + ], + httpClient: 'bun', + environment: { + SANDBOX_USE_CGROUPV2: 'false', + SANDBOX_REMOVE_UMOUNT_AFTER_STARTUP: 'false', + ...(fileRelayProfile + ? { + EGRESS_GATEWAY_URL: fileRelayProfile.url, + SANDBOX_PRIME_CONCURRENCY: String( + fileRelayLimits!.maxConcurrentRequests, + ), + SANDBOX_UPLOAD_CONCURRENCY: String( + fileRelayLimits!.maxConcurrentRequests, + ), + SANDBOX_FILE_RELAY_TOKEN: fileRelayProfile.token, + SANDBOX_REQUIRE_EGRESS_MANIFEST: 'true', + SANDBOX_EXECUTION_MANIFEST_PUBLIC_KEY: + executionManifestPublicKey!, + } + : {}), }, - ], - httpClient: 'bun', - environment: { - SANDBOX_USE_CGROUPV2: 'false', - SANDBOX_REMOVE_UMOUNT_AFTER_STARTUP: 'false', - }, - }; - })() - : {}), - }) - : new EndpointRuntimeSupervisor({ - endpoint: sandboxEndpoint, - statefulWorkspace, - }), - capabilities, - onIdentityChange: - pairedIdentity && identityPath - ? async (identity) => { - await saveBridgeIdentity(identityPath, { - ...pairedIdentity, - credential: identity.credential, - expiresAt: identity.expiresAt, - }); + }; + })() + : {}), + }) + : new EndpointRuntimeSupervisor({ + endpoint: sandboxEndpoint, + statefulWorkspace, + }), + capabilities, + onIdentityChange: + pairedIdentity && identityPath + ? async (identity) => { + await saveBridgeIdentity(identityPath, { + ...pairedIdentity, + credential: identity.credential, + expiresAt: identity.expiresAt, + }); + } + : undefined, + onRegistered: fileRelaySupervisor + ? async (registration) => { + if ( + registration.registrationGeneration == null || + !Number.isSafeInteger(registration.registrationGeneration) || + registration.registrationGeneration < 1 + ) { + throw new Error( + 'Code API does not support registration-ordered file relay activation', + ); + } + await fileRelaySupervisor.activate( + registration.registrationGeneration, + controller.signal, + ); } : undefined, - onError: (error) => { - const message = - error instanceof Error ? error.message : 'unknown bridge error'; - process.stderr.write(`librechat-code: reconnecting after ${message}\n`); - }, - }); - if (runtimeSessionId !== undefined) { - await worker.refreshCredential(controller.signal); - await worker.register(controller.signal); - await worker.resetWorkspace(runtimeSessionId, controller.signal); - process.stdout.write( - `librechat-code: reset acknowledged for ${runtimeSessionId}\n`, - ); - return; + onError: (error) => { + const message = + error instanceof Error ? error.message : 'unknown bridge error'; + process.stderr.write(`librechat-code: reconnecting after ${message}\n`); + }, + }); + if (runtimeSessionId !== undefined) { + await worker.refreshCredential(controller.signal); + await worker.register(controller.signal); + await worker.resetWorkspace(runtimeSessionId, controller.signal); + process.stdout.write( + `librechat-code: reset acknowledged for ${runtimeSessionId}\n`, + ); + return; + } + await worker.run(controller.signal); + } finally { + await fileRelaySupervisor?.stop(); } - await worker.run(controller.signal); } async function main(): Promise { const args = process.argv.slice(2); + if (args[0] === 'relay') { + await relay(); + return; + } if (args[0] === 'pair') { await pair(args); return; diff --git a/packages/code/src/protocol.ts b/packages/code/src/protocol.ts index b2027e1c..a9ff5649 100644 --- a/packages/code/src/protocol.ts +++ b/packages/code/src/protocol.ts @@ -11,6 +11,7 @@ export interface BridgeWorkerCapabilities { sandboxProfile: string; runtimes: string[]; policyDigest?: string; + requiresReadyConfirmation?: boolean; } export interface BridgeWorkerRegistration { @@ -24,6 +25,8 @@ export interface BridgeWorkerRegistrationResponse { protocolVersion: BridgeProtocolVersion; workerId: string; incarnationId: string; + /** Monotonic per-worker generation allocated when the active incarnation changes. */ + registrationGeneration?: number; registeredAt: string; leaseTtlMs: number; } @@ -138,6 +141,8 @@ export function isValidBridgeWorkerCapabilities( ) && (capabilities.policyDigest === undefined || (typeof capabilities.policyDigest === 'string' && - /^[a-f0-9]{64}$/.test(capabilities.policyDigest))) + /^[a-f0-9]{64}$/.test(capabilities.policyDigest))) && + (capabilities.requiresReadyConfirmation === undefined || + typeof capabilities.requiresReadyConfirmation === 'boolean') ); } diff --git a/packages/code/src/relay-runtime.test.ts b/packages/code/src/relay-runtime.test.ts new file mode 100644 index 00000000..bdbcf5f1 --- /dev/null +++ b/packages/code/src/relay-runtime.test.ts @@ -0,0 +1,506 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { DockerFileRelaySupervisor } from './relay-runtime.js'; + +import type { ContainerRuntimeClient } from './runtime.js'; + +test('Docker file relay prepares a private runtime network and hardened dual-homed relay', async () => { + const calls: string[][] = []; + let staleRelay = ''; + const client: ContainerRuntimeClient = { + async run(args) { + calls.push(args); + if (args[0] === 'network' && args[1] === 'inspect') { + throw new Error('network not found'); + } + if (args[0] === 'container' && args[1] === 'rm') { + throw new Error('No such container'); + } + if (args[0] === 'container' && args[1] === 'ls') return staleRelay; + if (args[0] === 'container' && args[1] === 'rename') return ''; + if (args[0] === 'network' && args[1] === 'create') return 'network-id\n'; + if (args[0] === 'run') return 'relay-id\n'; + if (args[0] === 'network' && args[1] === 'connect') return ''; + if (args[0] === 'exec') return '200'; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'incarnation-one', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + maxBytes: 20_000_000, + timeoutMs: 45_000, + maxConcurrentRequests: 4, + client, + }); + + const profile = await supervisor.prepare(); + + assert.match(profile.network, /^librechat-code-relay-/); + assert.equal(profile.url, 'http://relay:3000'); + assert.equal(profile.token, 'relay-secret'); + const networkCreates = calls.filter( + (args) => args[0] === 'network' && args[1] === 'create', + ); + assert.equal(networkCreates.length, 2); + assert.equal(networkCreates.filter((args) => args.includes('--internal')).length, 1); + const run = calls.find((args) => args[0] === 'run') ?? []; + const stagingRelay = run[run.indexOf('--name') + 1] ?? ''; + assert.match(stagingRelay, /^librechat-code-relay-.+-staging-[a-f0-9]{12}$/); + assert.match(run[run.indexOf('--network') + 1] ?? '', /^librechat-code-egress-/); + assert.ok(run.includes('LIBRECHAT_CODE_FILE_RELAY_MAX_BYTES=20000000')); + assert.ok(run.includes('LIBRECHAT_CODE_FILE_RELAY_TIMEOUT_MS=45000')); + assert.ok(run.includes('LIBRECHAT_CODE_FILE_RELAY_MAX_CONCURRENT_REQUESTS=4')); + assert.ok(run.includes('ALL')); + assert.ok(run.includes('no-new-privileges:true')); + assert.equal(run.includes('--publish'), false); + assert.equal( + calls.some((args) => args[0] === 'network' && args[1] === 'connect'), + false, + ); + staleRelay = stagingRelay.replace('-staging-', '-g1-'); + await supervisor.activate(2); + const rename = calls.find( + (args) => args[0] === 'container' && args[1] === 'rename', + ); + assert.match(rename?.at(-1) ?? '', /-g2-[a-f0-9]{12}$/); + const connect = calls.find( + (args) => args[0] === 'network' && args[1] === 'connect', + ); + assert.ok(connect?.includes('--alias')); + assert.ok(connect?.includes('relay')); + assert.ok(connect?.includes(profile.network)); + const health = calls.find((args) => args[0] === 'exec') ?? []; + assert.match(health.at(-1) ?? '', /^\d+$/); + assert.ok(calls.indexOf(health) < calls.indexOf(connect ?? [])); + + assert.ok( + calls.some( + (args) => + args[0] === 'container' && + args[1] === 'rm' && + args.includes(staleRelay), + ), + ); + const removalsBeforeStop = calls.filter( + (args) => args[0] === 'container' && args[1] === 'rm', + ).length; + await supervisor.stop(); + assert.equal( + calls.filter((args) => args[0] === 'container' && args[1] === 'rm').length, + removalsBeforeStop + 1, + ); +}); + +test('Docker file relay fails closed when a reused runtime network is not internal', async () => { + const client: ContainerRuntimeClient = { + async run(args) { + if (args[0] === 'network' && args[1] === 'inspect') { + return 'false|true|runtime\n'; + } + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'incarnation-two', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + client, + }); + + await assert.rejects(supervisor.prepare(), /does not match its required profile/); +}); + +test('Docker file relay validates a network created by a concurrent incarnation', async () => { + let runtimeInspections = 0; + const client: ContainerRuntimeClient = { + async run(args) { + if (args[0] === 'network' && args[1] === 'inspect') { + const isRuntime = args.at(-1)?.includes('-relay-') === true; + if (!isRuntime) return 'false|true|egress\n'; + runtimeInspections += 1; + if (runtimeInspections === 1) throw new Error('network not found'); + return 'true|true|runtime\n'; + } + if (args[0] === 'network' && args[1] === 'create') { + throw new Error('network with name already exists'); + } + if (args[0] === 'container' && args[1] === 'rm') { + throw new Error('No such container'); + } + if (args[0] === 'run') return 'relay-id\n'; + if (args[0] === 'network' && args[1] === 'connect') return ''; + if (args[0] === 'exec') return '200'; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'incarnation-three', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + client, + }); + + await supervisor.prepare(); + + assert.equal(runtimeInspections, 2); +}); + +test('Docker file relay rejects a delayed lower registration generation', async () => { + const calls: string[][] = []; + let currentRelay = ''; + let newerRelay = ''; + const client: ContainerRuntimeClient = { + async run(args) { + calls.push(args); + if (args[0] === 'network' && args[1] === 'inspect') { + return args.at(-1)?.includes('-egress-') === true + ? 'false|true|egress\n' + : 'true|true|runtime\n'; + } + if (args[0] === 'container' && args[1] === 'rm') { + return 'removed\n'; + } + if (args[0] === 'run') { + currentRelay = args[args.indexOf('--name') + 1] ?? ''; + newerRelay = currentRelay.replace( + /-staging-[a-f0-9]{12}$/, + '-g2-ffffffffffff', + ); + return 'relay-id\n'; + } + if (args[0] === 'container' && args[1] === 'rename') return ''; + if (args[0] === 'network' && args[1] === 'connect') return ''; + if (args[0] === 'exec') return '200'; + if (args[0] === 'container' && args[1] === 'ls') return `${newerRelay}\n`; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'older-incarnation', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + client, + }); + await supervisor.prepare(); + + await assert.rejects( + supervisor.activate(1), + /registration was superseded before activation/, + ); + + assert.ok( + calls.some( + (args) => + args[0] === 'container' && + args[1] === 'rm' && + args.includes(currentRelay.replace('-staging-', '-g1-')), + ), + ); + assert.equal( + calls.some( + (args) => args[0] === 'container' && args[1] === 'rm' && args.includes(newerRelay), + ), + false, + ); + assert.equal( + calls.some((args) => args[0] === 'network' && args[1] === 'connect'), + false, + ); +}); + +test('Docker file relay handoff follows registration order instead of creation order', async () => { + const containers = new Set(); + const connected: string[] = []; + const client: ContainerRuntimeClient = { + async run(args) { + if (args[0] === 'network' && args[1] === 'inspect') { + return args.at(-1)?.includes('-egress-') === true + ? 'false|true|egress\n' + : 'true|true|runtime\n'; + } + if (args[0] === 'container' && args[1] === 'rm') { + const name = args.at(-1) ?? ''; + if (!containers.delete(name)) throw new Error('No such container'); + return 'removed\n'; + } + if (args[0] === 'run') { + containers.add(args[args.indexOf('--name') + 1] ?? ''); + return 'relay-id\n'; + } + if (args[0] === 'exec') return '200'; + if (args[0] === 'container' && args[1] === 'rename') { + const previous = args.at(-2) ?? ''; + const next = args.at(-1) ?? ''; + if (!containers.delete(previous)) throw new Error('No such container'); + containers.add(next); + return ''; + } + if (args[0] === 'container' && args[1] === 'ls') { + return `${Array.from(containers) + .map((name) => `${name}|running|1000`) + .join('\n')}\n`; + } + if (args[0] === 'network' && args[1] === 'connect') { + const name = args.at(-1) ?? ''; + if (!containers.has(name)) throw new Error('No such container'); + connected.push(name); + return ''; + } + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const olderCreated = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'older-created-incarnation', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + now: () => 1_000, + client, + }); + const newerCreated = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'newer-created-incarnation', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + now: () => 1_000, + client, + }); + + await olderCreated.prepare(); + await newerCreated.prepare(); + await newerCreated.activate(1); + await olderCreated.activate(2); + + assert.equal(containers.size, 1); + assert.match(Array.from(containers)[0] ?? '', /-g2-[a-f0-9]{12}$/); + assert.match(connected[0] ?? '', /-g1-[a-f0-9]{12}$/); + assert.match(connected[1] ?? '', /-g2-[a-f0-9]{12}$/); +}); + +test('Docker file relay reclaims stopped and expired staging containers', async () => { + const removed: string[] = []; + let staging = ''; + let listCalls = 0; + const client: ContainerRuntimeClient = { + async run(args) { + if (args[0] === 'network' && args[1] === 'inspect') { + return args.at(-1)?.includes('-egress-') === true + ? 'false|true|egress\n' + : 'true|true|runtime\n'; + } + if (args[0] === 'container' && args[1] === 'rm') { + removed.push(args.at(-1) ?? ''); + return 'removed\n'; + } + if (args[0] === 'run') { + staging = args[args.indexOf('--name') + 1] ?? ''; + return 'relay-id\n'; + } + if (args[0] === 'exec') return '200'; + if (args[0] === 'container' && args[1] === 'rename') return ''; + if (args[0] === 'network' && args[1] === 'connect') return ''; + if (args[0] === 'container' && args[1] === 'ls') { + listCalls += 1; + if (listCalls === 1) return ''; + const prefix = staging.replace(/-staging-[a-f0-9]{12}$/, '-staging-'); + return [ + `${prefix}111111111111|exited|99000`, + `${prefix}222222222222|running|1`, + `${prefix}333333333333|running|99000`, + ].join('\n'); + } + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'current-incarnation', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + stagingGraceMs: 10_000, + now: () => 100_000, + client, + }); + + await supervisor.prepare(); + await supervisor.activate(1); + await supervisor.activate(1); + + assert.ok(removed.some((name) => name.endsWith('111111111111'))); + assert.ok(removed.some((name) => name.endsWith('222222222222'))); + assert.equal(removed.some((name) => name.endsWith('333333333333')), false); +}); + +test('Docker file relay relaunches an unhealthy active generation', async () => { + let relay = ''; + let launches = 0; + let healthChecks = 0; + const client: ContainerRuntimeClient = { + async run(args) { + if (args[0] === 'network' && args[1] === 'inspect') { + return args.at(-1)?.includes('-egress-') === true + ? 'false|true|egress\n' + : 'true|true|runtime\n'; + } + if (args[0] === 'container' && args[1] === 'rm') return 'removed\n'; + if (args[0] === 'run') { + launches += 1; + relay = args[args.indexOf('--name') + 1] ?? ''; + return 'relay-id\n'; + } + if (args[0] === 'exec') { + healthChecks += 1; + if (healthChecks === 2) throw new Error('container is not running'); + return '200'; + } + if (args[0] === 'container' && args[1] === 'rename') { + relay = args.at(-1) ?? ''; + return ''; + } + if (args[0] === 'container' && args[1] === 'ls') { + return `${relay}|running|1000\n`; + } + if (args[0] === 'network' && args[1] === 'connect') return ''; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'recovering-incarnation', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + client, + }); + + await supervisor.prepare(); + await supervisor.activate(1); + await supervisor.activate(1); + + assert.equal(launches, 2); + assert.ok(healthChecks >= 3); +}); + +test('Docker file relay relaunches when its live staging container was reclaimed', async () => { + let container = ''; + let launches = 0; + let renameAttempts = 0; + let connected = false; + const client: ContainerRuntimeClient = { + async run(args) { + if (args[0] === 'network' && args[1] === 'inspect') { + return args.at(-1)?.includes('-egress-') === true + ? 'false|true|egress\n' + : 'true|true|runtime\n'; + } + if (args[0] === 'container' && args[1] === 'rm') { + const name = args.at(-1) ?? ''; + if (name !== container || container === '') { + throw new Error('No such container'); + } + container = ''; + return 'removed\n'; + } + if (args[0] === 'run') { + launches += 1; + container = args[args.indexOf('--name') + 1] ?? ''; + return 'relay-id\n'; + } + if (args[0] === 'exec') return '200'; + if (args[0] === 'container' && args[1] === 'rename') { + renameAttempts += 1; + if (renameAttempts === 1) { + container = ''; + throw new Error('No such container'); + } + container = args.at(-1) ?? ''; + return ''; + } + if (args[0] === 'container' && args[1] === 'ls') { + return `${container}|running|1000\n`; + } + if (args[0] === 'network' && args[1] === 'connect') { + connected = true; + return ''; + } + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'delayed-incarnation', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + client, + }); + + await supervisor.prepare(); + await supervisor.activate(2); + + assert.equal(launches, 2); + assert.equal(renameAttempts, 2); + assert.equal(connected, true); + assert.match(container, /-g2-[a-f0-9]{12}$/); +}); + +test('Docker file relay rolls back startup with a fresh signal after abort', async () => { + const controller = new AbortController(); + let currentRelay = ''; + let cleanupCalls = 0; + let cleanupSignal: AbortSignal | undefined; + const client: ContainerRuntimeClient = { + async run(args, options) { + if (args[0] === 'network' && args[1] === 'inspect') { + return args.at(-1)?.includes('-egress-') === true + ? 'false|true|egress\n' + : 'true|true|runtime\n'; + } + if (args[0] === 'container' && args[1] === 'rm') { + if (args.includes(currentRelay)) { + cleanupCalls += 1; + cleanupSignal = options?.signal; + } + return 'removed\n'; + } + if (args[0] === 'run') { + currentRelay = args[args.indexOf('--name') + 1] ?? ''; + return 'relay-id\n'; + } + if (args[0] === 'network' && args[1] === 'connect') return ''; + if (args[0] === 'exec') { + controller.abort(new Error('shutdown')); + throw controller.signal.reason; + } + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerFileRelaySupervisor({ + workerId: 'engineering-vm', + incarnationId: 'aborted-incarnation', + image: 'librechat-code-worker:local', + upstreamUrl: 'https://code.example/egress', + token: 'relay-secret', + startupTimeoutMs: 1, + client, + }); + + await assert.rejects(supervisor.prepare(controller.signal), /shutdown/); + + assert.equal(cleanupCalls, 1); + assert.equal(cleanupSignal?.aborted ?? false, false); +}); diff --git a/packages/code/src/relay-runtime.ts b/packages/code/src/relay-runtime.ts new file mode 100644 index 00000000..889028a3 --- /dev/null +++ b/packages/code/src/relay-runtime.ts @@ -0,0 +1,425 @@ +import { createHash } from 'node:crypto'; + +import { validateFileRelayUpstream } from './relay.js'; +import { DockerCliClient } from './runtime.js'; + +import type { ContainerRuntimeClient } from './runtime.js'; + +export interface DockerFileRelaySupervisorOptions { + workerId: string; + incarnationId: string; + image: string; + upstreamUrl: string; + token: string; + maxBytes?: number; + timeoutMs?: number; + maxConcurrentRequests?: number; + dockerCommand?: string; + startupTimeoutMs?: number; + stagingGraceMs?: number; + now?: () => number; + client?: ContainerRuntimeClient; +} + +export interface DockerFileRelayProfile { + network: string; + url: string; + token: string; +} + +const DEFAULT_STARTUP_TIMEOUT_MS = 30_000; +const DEFAULT_STAGING_GRACE_MS = 5 * 60_000; +const TERMINAL_CONTAINER_STATES = new Set(['dead', 'exited']); + +function suffix(workerId: string): string { + return createHash('sha256').update(workerId).digest('hex').slice(0, 20); +} + +function missingNetwork(error: unknown): boolean { + return ( + error instanceof Error && + /(?:network(?: .*?)? not found|no such network)/i.test(error.message) + ); +} + +function existingNetwork(error: unknown): boolean { + return error instanceof Error && /network .* already exists/i.test(error.message); +} + +function missingContainer(error: unknown): boolean { + return ( + error instanceof Error && + /(?:no such container|no such object)/i.test(error.message) + ); +} + +export class DockerFileRelaySupervisor { + private readonly client: ContainerRuntimeClient; + private readonly network: string; + private readonly egressNetwork: string; + private readonly containerPrefix: string; + private readonly incarnationHash: string; + private readonly workerHash: string; + private container: string; + private prepared = false; + private activeGeneration?: number; + + constructor(private readonly options: DockerFileRelaySupervisorOptions) { + if (!options.image.trim()) { + throw new Error('Docker file relay image is required'); + } + if (!options.token.trim()) { + throw new Error('Docker file relay token is required'); + } + validateFileRelayUpstream(options.upstreamUrl); + for (const [name, value] of [ + ['maxBytes', options.maxBytes], + ['timeoutMs', options.timeoutMs], + ['maxConcurrentRequests', options.maxConcurrentRequests], + ['stagingGraceMs', options.stagingGraceMs], + ] as const) { + if (value != null && (!Number.isSafeInteger(value) || value <= 0)) { + throw new Error(`Docker file relay ${name} must be a positive integer`); + } + } + this.workerHash = suffix(options.workerId); + this.incarnationHash = suffix(options.incarnationId).slice(0, 12); + this.network = `librechat-code-relay-${this.workerHash}`; + this.egressNetwork = `librechat-code-egress-${this.workerHash}`; + this.containerPrefix = `librechat-code-relay-${this.workerHash}`; + this.container = this.stagingContainer(); + this.client = options.client ?? new DockerCliClient(options.dockerCommand); + } + + async prepare(signal?: AbortSignal): Promise { + await this.ensureNetwork(this.network, true, 'runtime', signal); + await this.ensureNetwork(this.egressNetwork, false, 'egress', signal); + await this.launchRelay(signal); + return { + network: this.network, + url: 'http://relay:3000', + token: this.options.token, + }; + } + + async stop(signal?: AbortSignal): Promise { + await this.removeRelay(signal); + } + + async activate( + registrationGeneration: number, + signal?: AbortSignal, + ): Promise { + if ( + !Number.isSafeInteger(registrationGeneration) || + registrationGeneration < 1 + ) { + throw new Error('Docker file relay registration generation is invalid'); + } + let activeAndHealthy = false; + if (this.activeGeneration === registrationGeneration) { + try { + await this.checkHealth(signal); + activeAndHealthy = true; + } catch { + await this.removeRelay(); + } + } + if (!this.prepared) await this.launchRelay(signal); + const activeContainer = `${this.containerPrefix}-g${registrationGeneration}-${this.incarnationHash}`; + if (this.container !== activeContainer) { + try { + await this.renameRelay(activeContainer, signal); + } catch (error) { + if (!missingContainer(error)) throw error; + this.resetPreparedState(); + await this.launchRelay(signal); + try { + await this.renameRelay(activeContainer, signal); + } catch (retryError) { + if (missingContainer(retryError)) this.resetPreparedState(); + throw retryError; + } + } + this.container = activeContainer; + } + const containers = await this.client.run( + [ + 'container', + 'ls', + '--all', + '--filter', + 'label=com.librechat.code.file-relay=true', + '--filter', + `label=com.librechat.code.worker-hash=${this.workerHash}`, + '--format', + '{{.Names}}|{{.State}}|{{.Label "com.librechat.code.file-relay-staged-at"}}', + ], + { signal }, + ); + const olderContainers: string[] = []; + for (const record of containers.split('\n').map((value) => value.trim())) { + const [name, state = '', stagedAtValue = ''] = record.split('|'); + if (!name || name === this.container) continue; + if (name.startsWith(`${this.containerPrefix}-staging-`)) { + const stagedAt = Number(stagedAtValue); + const staleStage = + Number.isSafeInteger(stagedAt) && + stagedAt > 0 && + (this.options.now?.() ?? Date.now()) - stagedAt >= + (this.options.stagingGraceMs ?? DEFAULT_STAGING_GRACE_MS); + if ( + TERMINAL_CONTAINER_STATES.has(state.toLowerCase()) || + staleStage + ) { + olderContainers.push(name); + } + continue; + } + const candidateGeneration = this.registrationGeneration(name); + if (candidateGeneration == null) { + throw new Error('Docker returned an invalid stale file relay name'); + } + if (candidateGeneration > registrationGeneration) { + await this.removeRelay(); + throw new Error( + 'Docker file relay registration was superseded before activation', + ); + } + if (candidateGeneration === registrationGeneration) { + throw new Error('Docker returned conflicting file relay registrations'); + } + olderContainers.push(name); + } + for (const name of olderContainers) { + try { + await this.client.run(['container', 'rm', '--force', name], { signal }); + } catch (error) { + if (!missingContainer(error)) throw error; + } + } + if (activeAndHealthy) return; + try { + await this.client.run( + [ + 'network', + 'connect', + '--alias', + 'relay', + this.network, + this.container, + ], + { signal }, + ); + } catch (error) { + if (missingContainer(error)) this.resetPreparedState(); + throw error; + } + this.activeGeneration = registrationGeneration; + } + + private registrationGeneration(name: string): number | undefined { + const match = new RegExp( + `^${this.containerPrefix}-g([1-9][0-9]*)-[a-f0-9]{12}$`, + ).exec(name); + if (match == null) return undefined; + const generation = Number(match[1]); + return Number.isSafeInteger(generation) ? generation : undefined; + } + + private stagingContainer(): string { + return `${this.containerPrefix}-staging-${this.incarnationHash}`; + } + + private async renameRelay( + activeContainer: string, + signal?: AbortSignal, + ): Promise { + await this.client.run( + ['container', 'rename', this.container, activeContainer], + { signal }, + ); + } + + private async launchRelay(signal?: AbortSignal): Promise { + this.container = this.stagingContainer(); + await this.removeRelay(signal); + this.container = this.stagingContainer(); + try { + await this.client.run( + [ + 'run', + '--detach', + '--name', + this.container, + '--network', + this.egressNetwork, + '--cap-drop', + 'ALL', + '--security-opt', + 'no-new-privileges:true', + '--read-only', + '--tmpfs', + '/tmp:rw,noexec,nosuid,size=16m', + '--label', + 'com.librechat.code.file-relay=true', + '--label', + `com.librechat.code.worker-hash=${this.workerHash}`, + '--label', + `com.librechat.code.file-relay-staged-at=${this.options.now?.() ?? Date.now()}`, + '--env', + `LIBRECHAT_CODE_FILE_RELAY_UPSTREAM=${this.options.upstreamUrl}`, + '--env', + `LIBRECHAT_CODE_FILE_RELAY_TOKEN=${this.options.token}`, + ...(this.options.maxBytes != null + ? [ + '--env', + `LIBRECHAT_CODE_FILE_RELAY_MAX_BYTES=${this.options.maxBytes}`, + ] + : []), + ...(this.options.timeoutMs != null + ? [ + '--env', + `LIBRECHAT_CODE_FILE_RELAY_TIMEOUT_MS=${this.options.timeoutMs}`, + ] + : []), + ...(this.options.maxConcurrentRequests != null + ? [ + '--env', + `LIBRECHAT_CODE_FILE_RELAY_MAX_CONCURRENT_REQUESTS=${this.options.maxConcurrentRequests}`, + ] + : []), + this.options.image, + 'relay', + ], + { signal }, + ); + await this.waitForHealth(signal); + this.prepared = true; + } catch (error) { + await this.removeRelay(); + throw error; + } + } + + private async ensureNetwork( + name: string, + internal: boolean, + role: 'runtime' | 'egress', + signal?: AbortSignal, + ): Promise { + try { + await this.validateNetwork(name, internal, role, signal); + } catch (error) { + if (!missingNetwork(error)) throw error; + try { + await this.client.run( + [ + 'network', + 'create', + ...(internal ? ['--internal'] : []), + '--label', + 'com.librechat.code.file-relay=true', + '--label', + `com.librechat.code.network-role=${role}`, + name, + ], + { signal }, + ); + } catch (createError) { + if (!existingNetwork(createError)) throw createError; + await this.validateNetwork(name, internal, role, signal); + } + } + } + + private async validateNetwork( + name: string, + internal: boolean, + role: 'runtime' | 'egress', + signal?: AbortSignal, + ): Promise { + const profile = await this.client.run( + [ + 'network', + 'inspect', + '--format', + '{{.Internal}}|{{index .Labels "com.librechat.code.file-relay"}}|{{index .Labels "com.librechat.code.network-role"}}', + name, + ], + { signal }, + ); + const expected = `${String(internal)}|true|${role}`; + if (profile.trim() !== expected) { + throw new Error( + `Docker file relay network ${name} does not match its required profile`, + ); + } + } + + private async removeRelay(signal?: AbortSignal): Promise { + try { + await this.client.run(['container', 'rm', '--force', this.container], { + signal, + }); + } catch (error) { + if (!missingContainer(error)) throw error; + } + this.resetPreparedState(); + } + + private resetPreparedState(): void { + this.container = this.stagingContainer(); + this.prepared = false; + this.activeGeneration = undefined; + } + + private async waitForHealth(signal?: AbortSignal): Promise { + const deadline = + Date.now() + + (this.options.startupTimeoutMs ?? DEFAULT_STARTUP_TIMEOUT_MS); + let lastError: unknown; + while (Date.now() < deadline) { + if (signal?.aborted) { + throw signal.reason ?? new DOMException('aborted', 'AbortError'); + } + try { + const remainingMs = Math.max(1, deadline - Date.now()); + await this.checkHealth(signal, remainingMs); + return; + } catch (error) { + if (signal?.aborted) { + throw signal.reason ?? new DOMException('aborted', 'AbortError'); + } + lastError = error; + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + throw new Error('Docker file relay did not become healthy', { + cause: lastError, + }); + } + + private async checkHealth( + signal?: AbortSignal, + timeoutMs = Math.max( + 1, + this.options.startupTimeoutMs ?? DEFAULT_STARTUP_TIMEOUT_MS, + ), + ): Promise { + const status = await this.client.run( + [ + 'exec', + this.container, + 'node', + '-e', + "fetch('http://127.0.0.1:3000/health',{headers:{'X-LibreChat-Code-Relay-Token':process.env.LIBRECHAT_CODE_FILE_RELAY_TOKEN},signal:AbortSignal.timeout(Number(process.argv.at(-1)))}).then(r=>process.stdout.write(String(r.status)))", + String(timeoutMs), + ], + { signal }, + ); + if (status.trim() !== '200') { + throw new Error(`File relay health check returned HTTP ${status.trim()}`); + } + } +} diff --git a/packages/code/src/relay.test.ts b/packages/code/src/relay.test.ts new file mode 100644 index 00000000..c0814408 --- /dev/null +++ b/packages/code/src/relay.test.ts @@ -0,0 +1,383 @@ +import assert from 'node:assert/strict'; +import { createServer } from 'node:http'; +import test from 'node:test'; + +import { startFileRelay } from './relay.js'; + +import type { AddressInfo } from 'node:net'; + +async function listen( + server: ReturnType, +): Promise { + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(0, '127.0.0.1', resolve); + }); + const address = server.address() as AddressInfo; + return `http://127.0.0.1:${address.port}`; +} + +test('file relay streams an authorized object download from its fixed upstream', async () => { + const upstream = createServer((req, res) => { + assert.equal(req.method, 'GET'); + assert.equal(req.url, '/sessions/storage-1/objects/file-1'); + assert.equal(req.headers['x-codeapi-egress-grant'], 'grant-1'); + res.writeHead(200, { + 'Content-Type': 'text/plain', + 'Content-Disposition': "attachment; filename*=UTF-8''canonical.txt", + 'X-Read-Only': 'true', + }); + res.end('input-data'); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const response = await fetch( + `${relay.url}/sessions/storage-1/objects/file-1`, + { + headers: { + 'X-LibreChat-Code-Relay-Token': 'relay-secret', + 'X-CodeAPI-Egress-Grant': 'grant-1', + }, + }, + ); + + assert.equal(response.status, 200); + assert.equal(response.headers.get('x-read-only'), 'true'); + assert.equal( + response.headers.get('content-disposition'), + "attachment; filename*=UTF-8''canonical.txt", + ); + assert.equal(await response.text(), 'input-data'); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay forwards an authorized generated-file upload', async () => { + const upstream = createServer(async (req, res) => { + const chunks: Buffer[] = []; + for await (const chunk of req) chunks.push(Buffer.from(chunk)); + assert.equal(req.method, 'PUT'); + assert.equal(req.url, '/sessions/output-1/objects/generated-1'); + assert.equal(req.headers['x-codeapi-egress-grant'], 'grant-2'); + assert.equal(req.headers['x-original-filename'], 'report.txt'); + assert.equal(Buffer.concat(chunks).toString('utf8'), 'generated-data'); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"stored":true}'); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const response = await fetch( + `${relay.url}/sessions/output-1/objects/generated-1`, + { + method: 'PUT', + headers: { + 'Content-Type': 'text/plain', + 'X-LibreChat-Code-Relay-Token': 'relay-secret', + 'X-CodeAPI-Egress-Grant': 'grant-2', + 'X-Original-Filename': 'report.txt', + }, + body: 'generated-data', + }, + ); + + assert.equal(response.status, 200); + assert.equal(await response.text(), '{"stored":true}'); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay permits only the normalized session object listing', async () => { + const upstream = createServer((req, res) => { + assert.equal(req.method, 'GET'); + assert.equal(req.url, '/sessions/storage-1/objects?detail=normalized'); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('[]'); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const response = await fetch( + `${relay.url}/sessions/storage-1/objects?detail=normalized`, + { + headers: { + 'X-LibreChat-Code-Relay-Token': 'relay-secret', + 'X-CodeAPI-Egress-Grant': 'grant-1', + }, + }, + ); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), []); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay health is available only with its private relay token', async () => { + const upstream = createServer((_req, res) => res.writeHead(500).end()); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const unauthorized = await fetch(`${relay.url}/health`); + assert.equal(unauthorized.status, 401); + const healthy = await fetch(`${relay.url}/health`, { + headers: { 'X-LibreChat-Code-Relay-Token': 'relay-secret' }, + }); + assert.equal(healthy.status, 200); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay rejects object traffic without an execution grant before contacting upstream', async () => { + let upstreamRequests = 0; + const upstream = createServer((_req, res) => { + upstreamRequests += 1; + res.writeHead(200).end('should-not-run'); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const response = await fetch( + `${relay.url}/sessions/storage-1/objects/file-1`, + { headers: { 'X-LibreChat-Code-Relay-Token': 'relay-secret' } }, + ); + assert.equal(response.status, 403); + assert.equal(upstreamRequests, 0); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay never follows upstream redirects', async () => { + let upstreamRequests = 0; + const upstream = createServer((req, res) => { + upstreamRequests += 1; + if (req.url === '/admin') { + res.writeHead(200).end('sensitive'); + return; + } + res.writeHead(302, { Location: '/admin' }).end(); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const response = await fetch( + `${relay.url}/sessions/storage-1/objects/file-1`, + { + redirect: 'manual', + headers: { + 'X-LibreChat-Code-Relay-Token': 'relay-secret', + 'X-CodeAPI-Egress-Grant': 'grant-1', + }, + }, + ); + assert.equal(response.status, 302); + assert.equal(upstreamRequests, 1); + assert.equal(response.headers.get('location'), null); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay bounds concurrent upstream transfers', async () => { + let releaseFirst: (() => void) | undefined; + const firstStarted = new Promise((resolve) => { + releaseFirst = resolve; + }); + let seen = 0; + const upstream = createServer(async (_req, res) => { + seen += 1; + if (seen === 1) await firstStarted; + res.writeHead(200).end('ok'); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + maxConcurrentRequests: 1, + }); + const headers = { + 'X-LibreChat-Code-Relay-Token': 'relay-secret', + 'X-CodeAPI-Egress-Grant': 'grant-1', + }; + + try { + const first = fetch(`${relay.url}/sessions/storage-1/objects/file-1`, { + headers, + }); + while (seen === 0) await new Promise((resolve) => setTimeout(resolve, 1)); + const second = await fetch( + `${relay.url}/sessions/storage-1/objects/file-2`, + { headers }, + ); + assert.equal(second.status, 503); + releaseFirst?.(); + assert.equal((await first).status, 200); + assert.equal(seen, 1); + } finally { + releaseFirst?.(); + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay rejects an oversized chunked upstream response', async () => { + const upstream = createServer((_req, res) => { + res.writeHead(200, { 'Content-Type': 'application/octet-stream' }); + res.write(Buffer.alloc(768)); + res.end(Buffer.alloc(768)); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const response = await fetch( + `${relay.url}/sessions/storage-1/objects/file-1`, + { + headers: { + 'X-LibreChat-Code-Relay-Token': 'relay-secret', + 'X-CodeAPI-Egress-Grant': 'grant-1', + }, + }, + ); + assert.equal(response.status, 502); + assert.equal(await response.text(), ''); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay accepts a valid scoped grant larger than Node defaults', async () => { + const grant = `grant-${'a'.repeat(32 * 1024)}`; + const upstream = createServer({ maxHeaderSize: 512 * 1024 }, (req, res) => { + assert.equal(req.headers['x-codeapi-egress-grant'], grant); + res.writeHead(200).end('ok'); + }); + const upstreamUrl = await listen(upstream); + const relay = await startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl, + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }); + + try { + const response = await fetch( + `${relay.url}/sessions/storage-1/objects/file-1`, + { + headers: { + 'X-LibreChat-Code-Relay-Token': 'relay-secret', + 'X-CodeAPI-Egress-Grant': grant, + }, + }, + ); + assert.equal(response.status, 200); + } finally { + await relay.close(); + await new Promise((resolve, reject) => + upstream.close((error) => (error ? reject(error) : resolve())), + ); + } +}); + +test('file relay rejects plaintext remote upstreams', async () => { + await assert.rejects( + startFileRelay({ + host: '127.0.0.1', + port: 0, + upstreamUrl: 'http://code.example/egress', + token: 'relay-secret', + maxBytes: 1024, + timeoutMs: 1_000, + }), + /HTTPS unless it is a local development host/, + ); +}); diff --git a/packages/code/src/relay.ts b/packages/code/src/relay.ts new file mode 100644 index 00000000..255845e9 --- /dev/null +++ b/packages/code/src/relay.ts @@ -0,0 +1,276 @@ +import { createHash, timingSafeEqual } from 'node:crypto'; +import { createServer } from 'node:http'; + +import type { IncomingMessage } from 'node:http'; +import type { AddressInfo } from 'node:net'; + +export interface FileRelayOptions { + host: string; + port: number; + upstreamUrl: string; + token: string; + maxBytes: number; + timeoutMs: number; + maxConcurrentRequests?: number; +} + +export interface FileRelayHandle { + url: string; + close(): Promise; +} + +const OBJECT_PATH = /^\/sessions\/[^/]+\/objects\/[^/]+$/; +const OBJECT_LIST_PATH = /^\/sessions\/[^/]+\/objects$/; +const MAX_RELAY_HEADER_BYTES = 512 * 1024; +const LOCAL_HTTP_HOSTS = new Set([ + '127.0.0.1', + '[::1]', + 'localhost', + 'host.docker.internal', + 'gateway.docker.internal', +]); + +class RelayPayloadTooLargeError extends Error {} +class UpstreamPayloadTooLargeError extends Error {} + +export function validateFileRelayUpstream(value: string): URL { + const upstream = new URL(value); + if ( + (upstream.protocol !== 'http:' && upstream.protocol !== 'https:') || + upstream.username || + upstream.password || + upstream.search || + upstream.hash + ) { + throw new Error( + 'File relay upstream must be an HTTP URL without credentials, query, or fragment', + ); + } + if ( + upstream.protocol !== 'https:' && + !LOCAL_HTTP_HOSTS.has(upstream.hostname.toLowerCase()) + ) { + throw new Error( + 'File relay upstream must use HTTPS unless it is a local development host', + ); + } + return upstream; +} + +function positiveInteger(name: string, value: number): number { + if (!Number.isSafeInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive integer`); + } + return value; +} + +function tokenMatches( + expected: string, + supplied: string | string[] | undefined, +): boolean { + if (typeof supplied !== 'string') return false; + return timingSafeEqual( + createHash('sha256').update(expected).digest(), + createHash('sha256').update(supplied).digest(), + ); +} + +async function readRequestBody( + request: IncomingMessage, + maxBytes: number, +): Promise { + const declaredLength = request.headers['content-length']; + if ( + typeof declaredLength === 'string' && + (!/^\d+$/.test(declaredLength) || Number(declaredLength) > maxBytes) + ) { + throw new RelayPayloadTooLargeError(); + } + const chunks: Buffer[] = []; + let bytes = 0; + for await (const chunk of request) { + const buffer = Buffer.from(chunk); + bytes += buffer.length; + if (bytes > maxBytes) throw new RelayPayloadTooLargeError(); + chunks.push(buffer); + } + return Buffer.concat(chunks); +} + +async function readResponseBody( + response: Response, + maxBytes: number, +): Promise { + const declaredLength = response.headers.get('content-length'); + if ( + declaredLength != null && + (!/^\d+$/.test(declaredLength) || Number(declaredLength) > maxBytes) + ) { + await response.body?.cancel(); + throw new UpstreamPayloadTooLargeError(); + } + if (response.body == null) return Buffer.alloc(0); + const reader = response.body.getReader(); + const chunks: Buffer[] = []; + let bytes = 0; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + bytes += value.byteLength; + if (bytes > maxBytes) throw new UpstreamPayloadTooLargeError(); + chunks.push(Buffer.from(value)); + } + } catch (error) { + await reader.cancel().catch(() => undefined); + throw error; + } + return Buffer.concat(chunks); +} + +export async function startFileRelay( + options: FileRelayOptions, +): Promise { + const upstream = validateFileRelayUpstream(options.upstreamUrl); + if (!options.token.trim()) throw new Error('File relay token is required'); + positiveInteger('File relay maxBytes', options.maxBytes); + positiveInteger('File relay timeoutMs', options.timeoutMs); + const maxConcurrentRequests = positiveInteger( + 'File relay maxConcurrentRequests', + options.maxConcurrentRequests ?? 8, + ); + let activeRequests = 0; + const server = createServer( + { maxHeaderSize: MAX_RELAY_HEADER_BYTES }, + async (request, response) => { + let admitted = false; + try { + if ( + !tokenMatches( + options.token, + request.headers['x-librechat-code-relay-token'], + ) + ) { + response.writeHead(401).end(); + return; + } + const requestUrl = new URL(request.url ?? '/', 'http://relay.invalid'); + if (request.method === 'GET' && requestUrl.pathname === '/health') { + response.writeHead(200, { 'Content-Type': 'application/json' }); + response.end('{"status":"ok"}'); + return; + } + const objectRequest = + OBJECT_PATH.test(requestUrl.pathname) && requestUrl.search.length === 0; + const normalizedListRequest = + request.method === 'GET' && + OBJECT_LIST_PATH.test(requestUrl.pathname) && + requestUrl.searchParams.size === 1 && + requestUrl.searchParams.get('detail') === 'normalized'; + if ( + (request.method !== 'GET' && request.method !== 'PUT') || + (!objectRequest && !normalizedListRequest) + ) { + response.writeHead(404).end(); + return; + } + const grant = request.headers['x-codeapi-egress-grant']; + if (typeof grant !== 'string' || grant.length === 0) { + response.writeHead(403).end(); + return; + } + if (activeRequests >= maxConcurrentRequests) { + response.writeHead(503, { 'Retry-After': '1' }).end(); + return; + } + activeRequests += 1; + admitted = true; + const target = new URL(upstream); + target.pathname = `${upstream.pathname.replace(/\/$/, '')}${ + requestUrl.pathname + }`; + target.search = requestUrl.search; + const requestBody = + request.method === 'PUT' + ? await readRequestBody(request, options.maxBytes) + : undefined; + const upstreamResponse = await fetch(target, { + method: request.method, + headers: { + ...(typeof grant === 'string' + ? { 'X-CodeAPI-Egress-Grant': grant } + : {}), + ...(request.method === 'PUT' + ? { + 'Content-Length': String(requestBody?.length ?? 0), + ...(typeof request.headers['content-type'] === 'string' + ? { + 'Content-Type': request.headers['content-type'], + } + : {}), + ...(typeof request.headers['x-original-filename'] === 'string' + ? { + 'X-Original-Filename': + request.headers['x-original-filename'], + } + : {}), + } + : {}), + }, + body: requestBody ? new Uint8Array(requestBody) : undefined, + redirect: 'manual', + signal: AbortSignal.timeout(options.timeoutMs), + }); + const body = await readResponseBody(upstreamResponse, options.maxBytes); + response.writeHead(upstreamResponse.status, { + ...(upstreamResponse.headers.get('content-type') + ? { + 'Content-Type': upstreamResponse.headers.get('content-type')!, + } + : {}), + ...(upstreamResponse.headers.get('x-read-only') + ? { + 'X-Read-Only': upstreamResponse.headers.get('x-read-only')!, + } + : {}), + ...(upstreamResponse.headers.get('content-disposition') + ? { + 'Content-Disposition': upstreamResponse.headers.get( + 'content-disposition', + )!, + } + : {}), + 'Content-Length': String(body.length), + }); + response.end(body); + } catch (error) { + if (error instanceof RelayPayloadTooLargeError) { + if (!response.headersSent) response.writeHead(413); + response.end(); + return; + } + if (!response.headersSent) response.writeHead(502); + response.end(); + } finally { + if (admitted) activeRequests -= 1; + } + }, + ); + server.requestTimeout = options.timeoutMs; + server.headersTimeout = options.timeoutMs; + server.keepAliveTimeout = Math.min(options.timeoutMs, 5_000); + server.maxRequestsPerSocket = 100; + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(options.port, options.host, resolve); + }); + const address = server.address() as AddressInfo; + return { + url: `http://${options.host}:${address.port}`, + close: async () => { + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())), + ); + }, + }; +} diff --git a/packages/code/src/runtime.test.ts b/packages/code/src/runtime.test.ts index 113f93bb..9ee67fae 100644 --- a/packages/code/src/runtime.test.ts +++ b/packages/code/src/runtime.test.ts @@ -1,4 +1,5 @@ import assert from 'node:assert/strict'; +import { createHash } from 'node:crypto'; import test from 'node:test'; import { DockerRuntimeSupervisor, EndpointRuntimeSupervisor } from './runtime.js'; @@ -109,6 +110,42 @@ test('docker runtime supervisor creates a networkless stateful runtime and execu assert.ok(health?.includes('--max-time')); }); +test('docker runtime supervisor preserves the legacy profile digest for the default network', async () => { + const image = 'example/code-runtime:latest'; + const legacyDigest = createHash('sha256') + .update( + JSON.stringify({ + version: 1, + image, + profileRevision: null, + restartStoppedContainers: true, + capabilities: [], + securityOptions: [], + environment: [], + bindMounts: [], + }), + ) + .digest('hex'); + const calls: string[][] = []; + const client: ContainerRuntimeClient = { + async run(args) { + calls.push(args); + if (args[0] === 'container' && args[1] === 'inspect') { + return `true|${legacyDigest}|sha256:image-1\n`; + } + if (args[0] === 'image' && args[1] === 'inspect') return 'sha256:image-1\n'; + if (args[0] === 'exec') return '200'; + throw new Error(`Unexpected Docker command: ${args.join(' ')}`); + }, + }; + const supervisor = new DockerRuntimeSupervisor({ image, client }); + + await supervisor.acquire(assignment('existing-workspace')); + + assert.equal(calls.some((args) => args[0] === 'container' && args[1] === 'rm'), false); + assert.equal(calls.some((args) => args[0] === 'run'), false); +}); + test('docker runtime supervisor applies an explicit macOS NsJail confinement profile', async () => { const calls: string[][] = []; const client: ContainerRuntimeClient = { @@ -123,6 +160,7 @@ test('docker runtime supervisor applies an explicit macOS NsJail confinement pro const supervisor = new DockerRuntimeSupervisor({ image: 'example/code-runtime:latest', client, + network: 'librechat-code-worker', capabilities: ['SYS_ADMIN', 'CHOWN'], securityOptions: ['seccomp=/repo/seccomp/nsjail.json'], environment: { SANDBOX_USE_CGROUPV2: 'false' }, @@ -134,6 +172,7 @@ test('docker runtime supervisor applies an explicit macOS NsJail confinement pro const run = calls.find(args => args[0] === 'run') ?? []; assert.ok(run.includes('SYS_ADMIN')); + assert.equal(run[run.indexOf('--network') + 1], 'librechat-code-worker'); assert.ok(run.includes('CHOWN')); assert.ok(run.includes('seccomp=/repo/seccomp/nsjail.json')); assert.ok(run.includes('SANDBOX_USE_CGROUPV2=false')); diff --git a/packages/code/src/runtime.ts b/packages/code/src/runtime.ts index bde50322..cf6eea48 100644 --- a/packages/code/src/runtime.ts +++ b/packages/code/src/runtime.ts @@ -47,6 +47,7 @@ export interface DockerRuntimeSupervisorOptions { image?: string; profileRevision?: string; restartStoppedContainers?: boolean; + network?: string; capabilities?: string[]; securityOptions?: string[]; environment?: Record; @@ -76,6 +77,7 @@ const DEFAULT_STARTUP_TIMEOUT_MS = 30_000; const DEFAULT_HEALTH_PATH = '/api/v2/health'; const CONTAINER_PREFIX = 'librechat-code-'; const CAPABILITY_PATTERN = /^[A-Z_]{1,32}$/; +const NETWORK_PATTERN = /^(?:none|[A-Za-z0-9][A-Za-z0-9_.-]{0,127})$/; const MAX_DOCKER_COMMAND_OUTPUT_BYTES = 64 * 1024 * 1024; function normalizedEndpoint(value: string): string { @@ -106,7 +108,7 @@ function isMissingImageError(error: unknown): boolean { return /(?:no such image|no such object)/i.test(error.message); } -class DockerCliClient implements ContainerRuntimeClient { +export class DockerCliClient implements ContainerRuntimeClient { private readonly command: string; constructor(command = 'docker') { @@ -165,6 +167,7 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { private readonly bindMounts: DockerRuntimeBindMount[]; private readonly httpClient: 'curl' | 'bun'; private readonly restartStoppedContainers: boolean; + private readonly network: string; constructor(private readonly options: DockerRuntimeSupervisorOptions) { if (options.image != null && options.image.trim().length === 0) { @@ -173,6 +176,9 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { if (options.capabilities?.some((capability) => !CAPABILITY_PATTERN.test(capability))) { throw new Error('Docker runtime capabilities must be uppercase capability names'); } + if (options.network != null && !NETWORK_PATTERN.test(options.network)) { + throw new Error('Docker runtime network name is invalid'); + } this.client = options.client ?? new DockerCliClient(options.dockerCommand); this.runnerPort = options.runnerPort ?? DEFAULT_RUNNER_PORT; this.startupTimeoutMs = options.startupTimeoutMs ?? DEFAULT_STARTUP_TIMEOUT_MS; @@ -183,6 +189,7 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { this.bindMounts = (options.bindMounts ?? []).map((mount) => ({ ...mount })); this.httpClient = options.httpClient ?? 'curl'; this.restartStoppedContainers = options.restartStoppedContainers ?? true; + this.network = options.network ?? 'none'; if ( this.bindMounts.some( ({ source, target }) => @@ -275,7 +282,7 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { '--name', name, '--network', - 'none', + this.network, '--cap-drop', 'ALL', ...this.capabilities.flatMap((capability) => ['--cap-add', capability]), @@ -310,6 +317,7 @@ export class DockerRuntimeSupervisor implements RuntimeSupervisor { image, profileRevision: this.options.profileRevision ?? null, restartStoppedContainers: this.restartStoppedContainers, + ...(this.network !== 'none' ? { network: this.network } : {}), capabilities: this.capabilities, securityOptions: this.securityOptions, environment: Object.entries(this.environment).sort(([left], [right]) => diff --git a/packages/code/src/worker.test.ts b/packages/code/src/worker.test.ts index f66074a3..68d95b28 100644 --- a/packages/code/src/worker.test.ts +++ b/packages/code/src/worker.test.ts @@ -12,6 +12,110 @@ import type { RuntimeSupervisor } from './runtime.js'; const incarnationId = 'incarnation-00000001'; +test('worker invokes lifecycle hooks only after its incarnation registers', async () => { + const registered: string[] = []; + const worker = new BridgeWorker({ + codeApiUrl: 'https://code.example/v1', + token: 'worker-secret', + workerId: 'vm-1', + incarnationId, + sandboxEndpoint: 'http://127.0.0.1:2000/api/v2', + capabilities: { + statefulWorkspace: false, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + }, + fetchImpl: async () => + Response.json({ + protocolVersion: 1, + workerId: 'vm-1', + incarnationId, + registeredAt: new Date().toISOString(), + leaseTtlMs: 60_000, + }), + onRegistered: async (registration) => { + registered.push(registration.incarnationId); + }, + }); + + await worker.register(); + + assert.deepEqual(registered, [incarnationId]); +}); + +test('worker confirms readiness only after local registration activation succeeds', async () => { + const events: string[] = []; + const worker = new BridgeWorker({ + codeApiUrl: 'https://code.example/v1', + token: 'worker-secret', + workerId: 'vm-1', + incarnationId, + sandboxEndpoint: 'http://127.0.0.1:2000/api/v2', + capabilities: { + statefulWorkspace: false, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + requiresReadyConfirmation: true, + }, + fetchImpl: async (input) => { + if (String(input).endsWith('/ready')) { + events.push('ready'); + return Response.json({ protocolVersion: 1, ready: true }); + } + events.push('registered'); + return Response.json({ + protocolVersion: 1, + workerId: 'vm-1', + incarnationId, + registrationGeneration: 3, + registeredAt: new Date().toISOString(), + leaseTtlMs: 60_000, + }); + }, + onRegistered: async () => { + events.push('activated'); + }, + }); + + await worker.register(); + + assert.deepEqual(events, ['registered', 'activated', 'ready']); +}); + +test('worker does not confirm readiness when local activation fails', async () => { + let readinessRequests = 0; + const worker = new BridgeWorker({ + codeApiUrl: 'https://code.example/v1', + token: 'worker-secret', + workerId: 'vm-1', + incarnationId, + sandboxEndpoint: 'http://127.0.0.1:2000/api/v2', + capabilities: { + statefulWorkspace: false, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + requiresReadyConfirmation: true, + }, + fetchImpl: async (input) => { + if (String(input).endsWith('/ready')) readinessRequests += 1; + return Response.json({ + protocolVersion: 1, + workerId: 'vm-1', + incarnationId, + registrationGeneration: 1, + registeredAt: new Date().toISOString(), + leaseTtlMs: 60_000, + }); + }, + onRegistered: async () => { + throw new Error('relay activation failed'); + }, + }); + + await assert.rejects(worker.register(), /relay activation failed/); + assert.equal(readinessRequests, 0); +}); + test('worker forwards a fenced assignment to the sandbox and settles the result', async () => { const requests: Array<{ url: string; init?: RequestInit }> = []; const fetchImpl: typeof fetch = async (input, init) => { diff --git a/packages/code/src/worker.ts b/packages/code/src/worker.ts index f4abfeba..81a2d91e 100644 --- a/packages/code/src/worker.ts +++ b/packages/code/src/worker.ts @@ -44,6 +44,9 @@ export interface BridgeWorkerOptions { fetchImpl?: typeof fetch; onError?: (error: unknown) => void; onIdentityChange?: (identity: BridgeWorkerIdentity) => void | Promise; + onRegistered?: ( + registration: BridgeWorkerRegistrationResponse, + ) => void | Promise; incarnationId?: string; } @@ -202,10 +205,43 @@ export class BridgeWorker { this.serverClockOffsetMs = registeredAtMs - registrationStartedAtMs; } this.registrationTtlMs = registration.leaseTtlMs; + await this.options.onRegistered?.(registration); + if (this.options.capabilities.requiresReadyConfirmation === true) { + await this.confirmReady(registration, signal); + } this.lastRegisteredAtMs = registrationStartedAtMs; return registration; } + private async confirmReady( + registration: BridgeWorkerRegistrationResponse, + signal?: AbortSignal, + ): Promise { + const registrationGeneration = registration.registrationGeneration; + if ( + !Number.isSafeInteger(registrationGeneration) || + (registrationGeneration ?? 0) < 1 + ) { + throw new BridgeProtocolError( + 'Code API does not support explicit worker readiness confirmation', + ); + } + await this.timedRequest( + `${this.codeApiUrl}${bridgeWorkerPath(this.options.workerId)}/ready`, + { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + incarnationId: this.incarnationId, + registrationGeneration, + }, + Math.max( + 1, + this.options.registrationTransportTimeoutMs ?? + DEFAULT_REGISTRATION_TRANSPORT_TIMEOUT_MS, + ), + signal, + ); + } + async resetWorkspace( runtimeSessionId: string, signal?: AbortSignal, diff --git a/service/src/bridge/pairing.ts b/service/src/bridge/pairing.ts index 74c75b7c..9eb6d76c 100644 --- a/service/src/bridge/pairing.ts +++ b/service/src/bridge/pairing.ts @@ -82,7 +82,7 @@ if credential then redis.call('DEL', KEYS[3]) redis.call('DEL', ARGV[1] .. credential) end -redis.call('DEL', KEYS[1], KEYS[3], KEYS[4], KEYS[5], KEYS[6]) +redis.call('DEL', KEYS[1], KEYS[3], KEYS[4], KEYS[5], KEYS[6], KEYS[7]) if activeIncarnation then redis.call('SET', ARGV[2] .. activeIncarnation .. ':fenced', '1') end @@ -446,13 +446,14 @@ export class RedisBridgePairingStore { // that linearizes afterward installs a distinct generation and code. await this.redis.eval( REVOKE_PAIRING_SCRIPT, - 6, + 7, workerPairingIndexKey(workerId), workerPairingGenerationKey(workerId), workerIdentityKey(workerId), workerStableIdentityKey(workerId), `${PREFIX}:worker:${encodeURIComponent(workerId)}`, `${PREFIX}:worker:${encodeURIComponent(workerId)}:incarnation`, + `${PREFIX}:worker:${encodeURIComponent(workerId)}:ready`, `${PREFIX}:credential:`, `${PREFIX}:worker:${encodeURIComponent(workerId)}:incarnation:`, ); diff --git a/service/src/bridge/router.test.ts b/service/src/bridge/router.test.ts index 2b3baa10..7e6e0fc5 100644 --- a/service/src/bridge/router.test.ts +++ b/service/src/bridge/router.test.ts @@ -278,6 +278,7 @@ describe('paired bridge HTTP API', () => { await expect(registrationResponse.json()).resolves.toMatchObject({ workerId: 'vm-1', incarnationId: 'incarnation-00000001', + registrationGeneration: 1, }); const crossDeploymentRevoke = await fetch( diff --git a/service/src/bridge/router.ts b/service/src/bridge/router.ts index e41336d4..996dd16b 100644 --- a/service/src/bridge/router.ts +++ b/service/src/bridge/router.ts @@ -362,10 +362,59 @@ router.post( : {}), }; try { - await options.store.register( + const registrationGeneration = await options.store.register( trustedRegistration, authorization, ); + res.json({ + protocolVersion: BRIDGE_PROTOCOL_VERSION, + workerId: registration.workerId, + incarnationId: registration.incarnationId, + registrationGeneration, + registeredAt: new Date().toISOString(), + leaseTtlMs: 60_000, + }); + } catch (error) { + if (error instanceof BridgeStoreError) { + sendStoreError(error, res); + return; + } + throw error; + } + }), +); + +router.post( + '/workers/:workerId/ready', + workerAuth, + asyncRoute(async (req, res) => { + const workerId = req.params.workerId; + const body = isRecord(req.body) ? req.body : {}; + if ( + !validWorkerId(workerId) || + body.protocolVersion !== BRIDGE_PROTOCOL_VERSION || + !validIncarnationId(body.incarnationId) || + !Number.isSafeInteger(body.registrationGeneration) || + Number(body.registrationGeneration) < 1 + ) { + res.status(400).json({ + error: 'Invalid bridge worker readiness confirmation', + }); + return; + } + if (!configuredWorker(workerId)) { + res.status(403).json({ + error: 'Worker is not authorized for this Code API deployment', + }); + return; + } + try { + await options.store.confirmReady( + workerId, + body.incarnationId, + Number(body.registrationGeneration), + ); + res.json({ protocolVersion: BRIDGE_PROTOCOL_VERSION, ready: true }); } catch (error) { if (error instanceof BridgeStoreError) { sendStoreError(error, res); @@ -373,13 +422,6 @@ router.post( } throw error; } - res.json({ - protocolVersion: BRIDGE_PROTOCOL_VERSION, - workerId: registration.workerId, - incarnationId: registration.incarnationId, - registeredAt: new Date().toISOString(), - leaseTtlMs: 60_000, - }); }), ); diff --git a/service/src/bridge/store.test.ts b/service/src/bridge/store.test.ts index 23b04067..e08273f9 100644 --- a/service/src/bridge/store.test.ts +++ b/service/src/bridge/store.test.ts @@ -6,6 +6,8 @@ import type * as t from '../types'; import { BRIDGE_PROTOCOL_VERSION } from '../../../packages/code/src/protocol'; import { RedisBridgeStore } from './store'; +import type { RegisteredBridgeWorker } from './store'; + const redis = new RedisMock() as unknown as Redis; const store = new RedisBridgeStore(redis); const incarnationId = 'incarnation-00000001'; @@ -13,12 +15,14 @@ const redisEval = redis.eval.bind(redis); const redisDel = redis.del.bind(redis); const redisLpop = redis.lpop.bind(redis); const redisGet = redis.get.bind(redis); +const redisMget = redis.mget.bind(redis); afterEach(async () => { redis.eval = redisEval as Redis['eval']; redis.del = redisDel as Redis['del']; redis.lpop = redisLpop as Redis['lpop']; redis.get = redisGet as Redis['get']; + redis.mget = redisMget as Redis['mget']; await redis.flushall(); }); @@ -72,7 +76,149 @@ describe('RedisBridgeStore', () => { runtimes: ['bash'], }, }, 'old-authenticated-credential-digest'), - ).resolves.toBeUndefined(); + ).resolves.toBe(1); + }); + + test('allocates registration generations only when the active incarnation changes', async () => { + const registration: RegisteredBridgeWorker = { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + workerId: 'generation-worker', + incarnationId, + capabilities: { + statefulWorkspace: true, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + }, + }; + + await expect(store.register(registration)).resolves.toBe(1); + await expect(store.register(registration)).resolves.toBe(1); + await expect( + store.register({ + ...registration, + incarnationId: 'incarnation-00000002', + }), + ).resolves.toBe(2); + }); + + test('dispatches an explicitly gated worker only after exact-generation readiness', async () => { + const workerId = 'ready-worker'; + const registration: RegisteredBridgeWorker = { + protocolVersion: BRIDGE_PROTOCOL_VERSION, + workerId, + incarnationId, + capabilities: { + statefulWorkspace: false, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + requiresReadyConfirmation: true, + }, + }; + const generation = await store.register(registration); + + await expect( + store.dispatch({ + workerId, + body: { language: 'bash' } as t.PayloadBody, + headers: {}, + deadlineAtMs: Date.now() + 1_000, + signal: new AbortController().signal, + }), + ).rejects.toMatchObject({ code: 'WORKER_OFFLINE' }); + + await store.confirmReady(workerId, incarnationId, generation); + const controller = new AbortController(); + const completion = store.dispatch({ + workerId, + body: { language: 'bash' } as t.PayloadBody, + headers: {}, + deadlineAtMs: Date.now() + 5_000, + signal: controller.signal, + }); + await expect(store.lease(workerId, incarnationId, 1_000)).resolves.toBeDefined(); + controller.abort(); + await expect(completion).rejects.toMatchObject({ code: 'ASSIGNMENT_EXPIRED' }); + + await store.register(registration); + const secondController = new AbortController(); + const secondCompletion = store.dispatch({ + workerId, + body: { language: 'bash' } as t.PayloadBody, + headers: {}, + deadlineAtMs: Date.now() + 5_000, + signal: secondController.signal, + }); + await expect(store.lease(workerId, incarnationId, 1_000)).resolves.toBeDefined(); + secondController.abort(); + await expect(secondCompletion).rejects.toMatchObject({ + code: 'ASSIGNMENT_EXPIRED', + }); + }); + + test('rejects readiness from a replaced registration generation', async () => { + const workerId = 'replaced-ready-worker'; + const capabilities = { + statefulWorkspace: false, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + requiresReadyConfirmation: true, + }; + const staleGeneration = await store.register({ + protocolVersion: BRIDGE_PROTOCOL_VERSION, + workerId, + incarnationId, + capabilities, + }); + await store.register({ + protocolVersion: BRIDGE_PROTOCOL_VERSION, + workerId, + incarnationId: 'incarnation-00000002', + capabilities, + }); + + await expect( + store.confirmReady(workerId, incarnationId, staleGeneration), + ).rejects.toMatchObject({ code: 'WORKER_FENCED' }); + }); + + test('does not enqueue after readiness is withdrawn during dispatch', async () => { + const workerId = 'readiness-race-worker'; + const generation = await store.register({ + protocolVersion: BRIDGE_PROTOCOL_VERSION, + workerId, + incarnationId, + capabilities: { + statefulWorkspace: false, + sandboxProfile: 'nsjail', + runtimes: ['bash'], + requiresReadyConfirmation: true, + }, + }); + await store.confirmReady(workerId, incarnationId, generation); + let withdrewReadiness = false; + redis.eval = (async (...args: Parameters) => { + if (!withdrewReadiness && String(args[0]).includes('ARGV[7]')) { + withdrewReadiness = true; + await redis.del(`codeapi:bridge:v1:worker:${workerId}:ready`); + } + return redisEval(...args); + }) as Redis['eval']; + + await expect( + store.dispatch({ + workerId, + body: { language: 'bash' } as t.PayloadBody, + headers: {}, + deadlineAtMs: Date.now() + 1_000, + signal: new AbortController().signal, + }), + ).rejects.toMatchObject({ code: 'WORKER_OFFLINE' }); + expect(withdrewReadiness).toBe(true); + await expect( + redis.llen( + `codeapi:bridge:v1:worker:${workerId}:incarnation:${incarnationId}:assignments`, + ), + ).resolves.toBe(0); }); test('rejects a dynamic worker lease outside its bound tenant', async () => { @@ -1174,7 +1320,7 @@ describe('RedisBridgeStore', () => { runtimes: [], }, }), - ).resolves.toBeUndefined(); + ).resolves.toBe(2); }); test('recovers only the assignment owner after registration expiry', async () => { @@ -1227,7 +1373,7 @@ describe('RedisBridgeStore', () => { runtimes: [], }, }), - ).resolves.toBeUndefined(); + ).resolves.toBe(1); controller.abort(); await expect(completion).rejects.toMatchObject({ @@ -1304,7 +1450,7 @@ describe('RedisBridgeStore', () => { runtimes: [], }, }); - redis.get = (() => new Promise(() => {})) as Redis['get']; + redis.mget = (() => new Promise<(string | null)[]>(() => {})) as Redis['mget']; await expect( timedStore.dispatch({ @@ -2072,6 +2218,6 @@ describe('RedisBridgeStore', () => { runtimes: [], }, }), - ).resolves.toBeUndefined(); + ).resolves.toBe(1); }); }); diff --git a/service/src/bridge/store.ts b/service/src/bridge/store.ts index 3a43298b..afc8a72b 100644 --- a/service/src/bridge/store.ts +++ b/service/src/bridge/store.ts @@ -68,6 +68,25 @@ function workerIncarnationKey(workerId: string): string { return `${PREFIX}:worker:${encodeURIComponent(workerId)}:incarnation`; } +function workerRegistrationGenerationKey(workerId: string): string { + return `${PREFIX}:worker:${encodeURIComponent(workerId)}:registration-generation`; +} + +function workerRegistrationGenerationIncarnationKey(workerId: string): string { + return `${PREFIX}:worker:${encodeURIComponent(workerId)}:registration-generation-incarnation`; +} + +function workerReadyKey(workerId: string): string { + return `${PREFIX}:worker:${encodeURIComponent(workerId)}:ready`; +} + +function workerReadyToken( + incarnationId: string, + registrationGeneration: number, +): string { + return `${incarnationId}:${registrationGeneration}`; +} + function incarnationFenceKey(workerId: string, incarnationId: string): string { return `${PREFIX}:worker:${encodeURIComponent(workerId)}:incarnation:${incarnationId}:fenced`; } @@ -240,7 +259,7 @@ export class RedisBridgeStore { pairingGeneration?: number; activeCredentialId?: string; }, - ): Promise { + ): Promise { const authorizationObject = typeof authorization === 'object' ? authorization : undefined; const expectedActiveCredentialId = @@ -278,15 +297,24 @@ export class RedisBridgeStore { ' redis.call(\'SET\', ARGV[4] .. current .. \':fenced\', \"1\")', ' end', 'end', + 'local registrationGeneration = tonumber(redis.call(\'GET\', KEYS[10]) or \"0\")', + 'local registrationGenerationIncarnation = redis.call(\'GET\', KEYS[11])', + 'local registrationGenerationChanged = false', + 'if registrationGeneration < 1 or registrationGenerationIncarnation ~= ARGV[1] then', + ' registrationGeneration = redis.call(\'INCR\', KEYS[10])', + ' redis.call(\'SET\', KEYS[11], ARGV[1])', + ' registrationGenerationChanged = true', + 'end', 'redis.call(\'SET\', KEYS[1], ARGV[2], \"EX\", ARGV[3])', 'redis.call(\'SET\', KEYS[4], ARGV[1], \"EX\", ARGV[3])', - 'return 1', + 'if ARGV[9] == "1" and registrationGenerationChanged then redis.call(\'DEL\', KEYS[12]) end', + 'return registrationGeneration', ].join('\n'); const result = Number( await boundedCommand( this.redis.eval( script, - 9, + 12, workerKey(registration.workerId), incarnationFenceKey(registration.workerId, registration.incarnationId), quarantineKey(registration.workerId, registration.incarnationId), @@ -296,6 +324,9 @@ export class RedisBridgeStore { `${PREFIX}:pairing-generation:${registration.workerId}`, `${PREFIX}:stable-identity:${registration.workerId}`, `${PREFIX}:identity:${registration.workerId}`, + workerRegistrationGenerationKey(registration.workerId), + workerRegistrationGenerationIncarnationKey(registration.workerId), + workerReadyKey(registration.workerId), registration.incarnationId, JSON.stringify(registration), String(this.workerTtlSeconds), @@ -306,6 +337,7 @@ export class RedisBridgeStore { authorizationObject?.identityId ?? '', expectedActiveCredentialId ?? '', registration.identityId ?? '', + registration.capabilities.requiresReadyConfirmation === true ? '1' : '0', ), this.redisCommandTimeoutMs, 'Bridge worker registration', @@ -341,6 +373,73 @@ export class RedisBridgeStore { 'Bridge worker authorization was revoked before registration completed', ); } + if (!Number.isSafeInteger(result) || result < 1) { + throw new Error('Bridge worker registration returned an invalid generation'); + } + return result; + } + + async confirmReady( + workerId: string, + incarnationId: string, + registrationGeneration: number, + ): Promise { + const result = Number( + await boundedCommand( + this.redis.eval( + [ + 'if redis.call(\'EXISTS\', KEYS[1]) == 0 then return -1 end', + 'if redis.call(\'GET\', KEYS[2]) ~= ARGV[1] then return -2 end', + 'if redis.call(\'GET\', KEYS[3]) ~= ARGV[2] then return -2 end', + 'if redis.call(\'GET\', KEYS[4]) ~= ARGV[1] then return -2 end', + 'if redis.call(\'EXISTS\', KEYS[5]) == 1 then return -2 end', + 'if redis.call(\'EXISTS\', KEYS[6]) == 1 then return -3 end', + 'redis.call(\'SET\', KEYS[7], ARGV[3], "EX", ARGV[4])', + 'return 1', + ].join('\n'), + 7, + workerKey(workerId), + workerIncarnationKey(workerId), + workerRegistrationGenerationKey(workerId), + workerRegistrationGenerationIncarnationKey(workerId), + incarnationFenceKey(workerId, incarnationId), + quarantineKey(workerId, incarnationId), + workerReadyKey(workerId), + incarnationId, + String(registrationGeneration), + workerReadyToken(incarnationId, registrationGeneration), + String( + Math.min( + this.workerTtlSeconds, + Math.ceil(this.workerTtlSeconds / 2) + 5, + ), + ), + ), + this.redisCommandTimeoutMs, + 'Bridge worker readiness confirmation', + ), + ); + if (result === -1) { + throw new BridgeStoreError( + 'WORKER_OFFLINE', + 'Bridge worker registration expired before readiness confirmation', + ); + } + if (result === -2) { + throw new BridgeStoreError( + 'WORKER_FENCED', + 'Bridge worker readiness confirmation is stale', + ); + } + if (result === -3) { + throw new BridgeStoreError( + 'WORKER_QUARANTINED', + 'Bridge worker incarnation is quarantined', + ); + } + if (result !== 1) { + throw new Error('Bridge worker readiness confirmation failed'); + } } async dispatch(args: { @@ -357,17 +456,18 @@ export class RedisBridgeStore { ) => Promise; }): Promise { this.assertDispatchActive(args.signal, args.deadlineAtMs); - let registration = await this.dispatchCommand( - () => this.registration(args.workerId), + const dispatchable = await this.dispatchCommand( + () => this.dispatchableRegistration(args.workerId), args, 'Bridge worker registration read', ); - if (registration == null) { + if (dispatchable == null) { throw new BridgeStoreError( 'WORKER_OFFLINE', `Bridge worker ${args.workerId} is offline`, ); } + let { registration, readyToken } = dispatchable; if ( (args.requireTenantBinding === true && registration.binding == null) || (registration.binding != null && @@ -459,13 +559,18 @@ export class RedisBridgeStore { this.assertDispatchActive(args.signal, args.deadlineAtMs); assignment.incarnationId = registration.incarnationId; queued = await this.dispatchCommand( - () => this.enqueueForActiveIncarnation(assignment!, ttlSeconds), + () => + this.enqueueForActiveIncarnation( + assignment!, + ttlSeconds, + readyToken, + ), args, 'Bridge assignment enqueue', ); if (queued) break; const replacement = await this.dispatchCommand( - () => this.registration(args.workerId), + () => this.dispatchableRegistration(args.workerId), args, 'Bridge replacement registration read', ); @@ -477,14 +582,15 @@ export class RedisBridgeStore { } if ( args.runtimeSessionId !== undefined && - replacement.capabilities.statefulWorkspace !== true + replacement.registration.capabilities.statefulWorkspace !== true ) { throw new BridgeStoreError( 'WORKER_MISMATCH', `Bridge worker ${args.workerId} does not provide a stateful workspace`, ); } - registration = replacement; + registration = replacement.registration; + readyToken = replacement.readyToken; } if (!queued) { throw new BridgeStoreError( @@ -1103,6 +1209,35 @@ export class RedisBridgeStore { return raw == null ? undefined : (JSON.parse(raw) as RegisteredBridgeWorker); } + private async dispatchableRegistration( + workerId: string, + ): Promise< + | { registration: RegisteredBridgeWorker; readyToken?: string } + | undefined + > { + const [raw, ready, generation, generationIncarnation] = await this.redis.mget( + workerKey(workerId), + workerReadyKey(workerId), + workerRegistrationGenerationKey(workerId), + workerRegistrationGenerationIncarnationKey(workerId), + ); + if (raw == null) return undefined; + const registration = JSON.parse(raw) as RegisteredBridgeWorker; + if (registration.capabilities.requiresReadyConfirmation !== true) { + return { registration }; + } + const registrationGeneration = Number(generation); + if ( + !Number.isSafeInteger(registrationGeneration) || + registrationGeneration < 1 || + generationIncarnation !== registration.incarnationId || + ready !== workerReadyToken(registration.incarnationId, registrationGeneration) + ) { + return undefined; + } + return { registration, readyToken: ready }; + } + private assertDispatchActive( signal: AbortSignal, deadlineAtMs: number, @@ -1201,16 +1336,18 @@ export class RedisBridgeStore { private async enqueueForActiveIncarnation( assignment: StoredAssignment, ttlSeconds: number, + readyToken?: string, ): Promise { const script = [ 'if redis.call(\'GET\', KEYS[1]) ~= ARGV[1] then return 0 end', - 'if #KEYS == 6 and redis.call(\'EXISTS\', KEYS[6]) == 1 then return -1 end', + 'if ARGV[7] ~= "" and redis.call(\'GET\', KEYS[6]) ~= ARGV[7] then return 0 end', + 'if #KEYS == 7 and redis.call(\'EXISTS\', KEYS[7]) == 1 then return -1 end', 'redis.call(\'SET\', KEYS[2], ARGV[2], \"EX\", ARGV[3])', 'redis.call(\'RPUSH\', KEYS[3], ARGV[4])', 'redis.call(\'EXPIRE\', KEYS[3], ARGV[3])', 'redis.call(\'SET\', KEYS[4], ARGV[1], \"PX\", ARGV[5])', 'redis.call(\'SET\', KEYS[5], "1", \"PXAT\", ARGV[6])', - 'if #KEYS == 6 then redis.call(\'SET\', KEYS[6], ARGV[4]) end', + 'if #KEYS == 7 then redis.call(\'SET\', KEYS[7], ARGV[4]) end', 'return 1', ].join('\n'); const keys = [ @@ -1219,6 +1356,7 @@ export class RedisBridgeStore { queueKey(assignment.workerId, assignment.incarnationId), lockIncarnationKey(assignment.workerId), assignmentDeadlineKey(assignment.assignmentId), + workerReadyKey(assignment.workerId), ]; if (assignment.runtimeSessionId !== undefined) { keys.push( @@ -1238,6 +1376,7 @@ export class RedisBridgeStore { assignment.assignmentId, String(ttlSeconds * 1000), String(Date.parse(assignment.expiresAt)), + readyToken ?? '', ); if (Number(result) === -1) { throw new BridgeStoreError( From 0739f3d6397083c7d76a8779522f758020136d14 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 12:49:15 -0400 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20fix:=20Make=20Code?= =?UTF-8?q?=20Environment=20File=20Deletion=20Work=20(#85)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🗑️ fix: Make Code Environment File Deletion Work Object deletion has never removed anything, and the failure was silent at every layer. The client (LibreChat `deleteCodeEnvFile`) issues DELETE against `/v1/sessions/:session_id/objects/:fileId`, the file-server's own path, which is not exposed on `/v1` — only GET is mounted there. Every deletion 404'd, and a 404 is indistinguishable from "already gone", so the caller cleared its state and the bucket only ever grew (13 GiB / 29k objects on a six-week-old deployment, per danny-avila/LibreChat#15511). Mount DELETE on that path as an alias of `/v1/files/:session_id/:fileId`, so deployments running a client older than LibreChat v0.8.6 — before the fallback to `/files/...` landed — delete successfully. Pass the file-server's 404 through instead of collapsing it into a 500: a 500 reads as retryable, and a client sweeping its retention window re-issues the same DELETE hourly, forever, for an object that no longer exists. Correcting the route is not sufficient on its own. `sessionAuth` authorizes deletion against `session:`, whose `SESSION_CACHE_TTL` is 24h and is not refreshed by use, so an object was deletable only for the day following upload and stranded permanently after that — unreadable, unusable as an execution input, and undeletable through every route. Clients are typically far outside that window when they get there; LibreChat's default retention is 30 days. Record ownership twice: `session:` stays the hot-path cache bounding read access, and a durable `session-owner:` record (`SESSION_OWNER_TTL`, 90 days, never shorter than the cache TTL) backs deletion once the cache key has lapsed. The fallback applies to DELETE only — reads keep the window they have always had — and a live cache key naming a different owner remains authoritative, so a re-registered session is never deletable by its previous owner. The recovery script restores both records, so a rehydrated session stays deletable rather than stranding again a day later. * fix: Close Codex review findings on session ownership Four P2 findings from the review of daae56c: - The 404 deletion path cleared the upload key with a bare `await` inside the catch block. A Redis failure there rejects with no handler above it, and Express 4 does not forward async rejections, so the request would hang instead of answering 404. Make the cleanup best effort and log it. - The blocking PTC path discarded the registration promise with `void`, preserving the previous fire-and-forget behavior. That now spans two keys: a partial write (cache key stored, durable record refused by a Redis ACL scoped to `session:*`) would produce exactly the undeletable files this change exists to prevent. Await it; the caller turns a rejection into a 500 before anything is enqueued. - Recovery treated a durable owner record naming someone else as a log line while still counting the session as restored or matching, so an apply could exit 0 having recovered nothing usable. Reconcile the owner record before touching the cache key and report the disagreement as a conflict, in dry run as well as apply. The cache key is no longer restored for those sessions either — the manifest's claimant should not get a day of access the service never granted it. - `SET NX` cannot extend an expiry, so a matching owner record could carry less remaining TTL than the cache key being restored and lapse first, stranding the session again just as recovery reported success. Top up the expiry when it is shorter than the target, leaving longer ones alone. * fix: Settle session ownership before recovery writes anything Two findings from the review of 816dbb5: - Reconciling the durable owner record first meant creating it before the live cache key had been consulted. For a session whose durable record was absent and whose cache key named a different owner, recovery wrote a durable record for the manifest's claimant, then reported the cache conflict and moved on — leaving the record behind. It outlives the cache key by design, so once that expired, `sessionAuth` would authorize the manifest owner to delete the real owner's files. Split the read from the write. The durable record is now inspected read-only up front, where a disagreement still settles the session before anything is written, and is created or extended only once the cache key has been confirmed to name the same owner. - `/exec` registered ownership before entering the route's `try`. Express 4 does not forward a rejected async handler to the error middleware, so a Redis failure there would hang the request rather than answering. Guard it and return a controlled 500. * fix: Harden recovery's durable owner handling Three findings from the review of 47b36e0, all in the recovery script: - A `SET NX` that lost the race to a key which then expired before the follow-up read left no record and no conflict, and the session was reported as recovered while its durable half did not exist. Retry once, and report anything past that as missing so an apply exits nonzero instead of claiming success. - When the owner commit conflicted on a session whose cache key this run had just created, the cache key stayed. That grant authorizes reads and deletes for its full TTL while the durable record names somebody else, so roll it back. A durable record that merely could not be created is left alone: the session is no worse off than before the run, and removing the grant would leave the operator with nothing. - A dry run reported a session whose cache key already matched as `matching` even when its owner record was absent or short-lived, hiding the work an apply would do and inviting operators to skip it. Pending owner repairs now count as missing. * fix: Answer Redis failures instead of hanging on them Two findings from the review of d1de664: - `sessionAuth` awaited the ownership lookup unguarded. Express 4 does not forward a rejected async middleware, so an unavailable Redis — or an ACL granting `session:*` but not `session-owner:*` — would hang a DELETE rather than answering it. Catch and return a controlled 500. - The recovery script's TTL top-up read, extended and returned across three round trips, reporting success on evidence it had not rechecked. The record can lapse in between, in which case it is now created fresh, or name somebody else, in which case the session is a conflict. Extending a record that turns out to belong to another owner prolongs a claim the service itself wrote and grants nothing new, but reporting the session as recovered on that basis would not be true. --- service/scripts/rehydrate-session-cache.ts | 226 ++++++++++++- service/src/config.ts | 10 + service/src/middleware/auth.ts | 32 +- service/src/rehydrate-session-cache.test.ts | 337 +++++++++++++++++++- service/src/service/programmatic-router.ts | 12 +- service/src/service/router.ts | 62 +++- service/src/session-ownership.test.ts | 165 ++++++++++ service/src/session-ownership.ts | 126 ++++++++ 8 files changed, 950 insertions(+), 20 deletions(-) create mode 100644 service/src/session-ownership.test.ts create mode 100644 service/src/session-ownership.ts diff --git a/service/scripts/rehydrate-session-cache.ts b/service/scripts/rehydrate-session-cache.ts index c21afa92..819bb250 100644 --- a/service/scripts/rehydrate-session-cache.ts +++ b/service/scripts/rehydrate-session-cache.ts @@ -16,10 +16,18 @@ import { redisKeepAliveOptions } from '../src/redis-options'; * {"type":"source","environment":"example","region":"region-1","namespace":"codeapi","query_start_utc":"2026-01-01T00:00:00Z","query_end_utc":"2026-01-02T00:00:00Z"} * {"session_id":"<21-character id>","expected_session_key":""} * - * The apply path uses SET NX and never replaces an existing owner. + * The apply path uses SET NX and never replaces an existing owner. Both the + * `session:` cache key and the durable `session-owner:` record are + * restored, so recovered sessions stay deletable past SESSION_CACHE_TTL + * rather than stranding again a day later. A durable owner record naming a + * different owner is reported as a conflict and the session is skipped + * entirely, in both dry-run and apply. */ const DEFAULT_SESSION_CACHE_TTL_SECONDS = 86400; +const DEFAULT_SESSION_OWNER_TTL_SECONDS = 90 * 86400; +/** Redis `TTL` reply for a key that does not exist. */ +const KEY_ABSENT_TTL = -2; const MAX_RECOVERY_CONTEXT_LENGTH = 128; const MAX_RECOVERY_SESSION_KEY_LENGTH = 512; const MAX_RECONNECT_ATTEMPTS = 5; @@ -48,6 +56,9 @@ export interface RecoveryStore { ttlSeconds: number, condition: 'NX', ): Promise<'OK' | null>; + ttl(key: string): Promise; + expire(key: string, ttlSeconds: number): Promise; + del(key: string): Promise; } export interface RecoverySummary { @@ -78,6 +89,7 @@ interface Options extends RecoveryScope { apply: boolean; inputPath?: string; ttlSeconds: number; + ownerTtlSeconds: number; } function usage(): string { @@ -97,8 +109,15 @@ Options: SESSION_RECOVERY_REGION. --namespace Expected source namespace. Defaults to SESSION_RECOVERY_NAMESPACE. - --ttl-seconds Redis TTL for restored keys. Defaults to - SESSION_CACHE_TTL or ${DEFAULT_SESSION_CACHE_TTL_SECONDS}. + --ttl-seconds Redis TTL for restored session cache keys. + Defaults to SESSION_CACHE_TTL or + ${DEFAULT_SESSION_CACHE_TTL_SECONDS}. + --owner-ttl-seconds + Redis TTL for the durable session-owner records + restored alongside them. Defaults to + SESSION_OWNER_TTL or + ${DEFAULT_SESSION_OWNER_TTL_SECONDS}, and is never + shorter than --ttl-seconds. --help Show this help. Keep recovery manifests outside the repository because expected_session_key @@ -154,6 +173,10 @@ export function parseOptions(args: string[], env: NodeJS.ProcessEnv = process.en let ttlSeconds = configuredTtl != null && configuredTtl !== '' ? parsePositiveInteger(configuredTtl, 'SESSION_CACHE_TTL') : DEFAULT_SESSION_CACHE_TTL_SECONDS; + const configuredOwnerTtl = env.SESSION_OWNER_TTL?.trim(); + let ownerTtlSeconds = configuredOwnerTtl != null && configuredOwnerTtl !== '' + ? parsePositiveInteger(configuredOwnerTtl, 'SESSION_OWNER_TTL') + : DEFAULT_SESSION_OWNER_TTL_SECONDS; let environment = env.SESSION_RECOVERY_ENVIRONMENT; let region = env.SESSION_RECOVERY_REGION; let namespace = env.SESSION_RECOVERY_NAMESPACE; @@ -189,6 +212,13 @@ export function parseOptions(args: string[], env: NodeJS.ProcessEnv = process.en ); index += 1; break; + case '--owner-ttl-seconds': + ownerTtlSeconds = parsePositiveInteger( + optionValue(args, index, '--owner-ttl-seconds'), + '--owner-ttl-seconds', + ); + index += 1; + break; case '--help': break; default: @@ -203,6 +233,9 @@ export function parseOptions(args: string[], env: NodeJS.ProcessEnv = process.en region: parseRecoveryContext(region, 'Recovery region'), namespace: parseRecoveryContext(namespace, 'Recovery namespace'), ttlSeconds, + /* The durable record is what keeps a recovered session deletable + * beyond the cache TTL, so it can never be the shorter of the two. */ + ownerTtlSeconds: Math.max(ownerTtlSeconds, ttlSeconds), }; } @@ -320,11 +353,141 @@ export function parseRecoveryManifest( return { source, records: [...bySessionId.values()] }; } +type OwnerInspection = + | { status: 'agrees'; existing: string | null } + | { status: 'conflict' }; + +/** + * Reads the durable owner record without writing. A record naming a + * different owner is the strongest ownership signal available, so it + * settles the session before anything is restored. + */ +async function inspectOwnerRecord( + store: RecoveryStore, + record: RecoveryRecord, +): Promise { + const existing = await store.get(`session-owner:${record.session_id}`); + if (existing !== null && existing !== record.expected_session_key) { + return { status: 'conflict' }; + } + return { status: 'agrees', existing }; +} + +/** + * Extends the durable record when it would lapse before the cache key this + * run just restored. `SET NX` cannot do it: a record near the end of its + * life would otherwise strand the session again the moment recovery + * reported success. + * + * The read, the extension and the confirmation are three round trips, so + * the record is re-read afterwards rather than assumed: it can expire in + * between (the caller then creates it fresh) or name somebody else (a + * conflict). Extending a record that turns out to belong to another owner + * prolongs a claim the service itself wrote and grants nothing new, but + * reporting the session as recovered on that basis would be a lie. + */ +async function refreshOwnerTtl( + store: RecoveryStore, + ownerKey: string, + record: RecoveryRecord, + ownerTtlSeconds: number, +): Promise<'ready' | 'conflict' | 'vanished'> { + const remaining = await store.ttl(ownerKey); + if (remaining === KEY_ABSENT_TTL) { + return 'vanished'; + } + if (remaining >= 0 && remaining < ownerTtlSeconds && await store.expire(ownerKey, ownerTtlSeconds) === 0) { + return 'vanished'; + } + + const confirmed = await store.get(ownerKey); + if (confirmed === null) { + return 'vanished'; + } + return confirmed === record.expected_session_key ? 'ready' : 'conflict'; +} + +/** + * Creates or extends the durable owner record for a session whose cache + * key has just been confirmed to name the same owner. Runs only after that + * confirmation: writing it earlier would leave a record behind for a + * manifest owner the live cache key contradicts, and `sessionAuth` would + * later authorize deletion through it. + */ +async function ensureOwnerRecord( + store: RecoveryStore, + record: RecoveryRecord, + ownerTtlSeconds: number, + existing: string | null, +): Promise<'ready' | 'conflict' | 'unresolved'> { + const ownerKey = `session-owner:${record.session_id}`; + + if (existing !== null) { + const refreshed = await refreshOwnerTtl(store, ownerKey, record, ownerTtlSeconds); + if (refreshed !== 'vanished') { + return refreshed; + } + /* Expired between the inspection and the refresh. Fall through and + * create it as though it had never been there. */ + } + + /* `SET NX` can lose to a writer whose key then expires before the + * follow-up read, leaving no record and no conflict to report. One retry + * settles that; anything past it is reported rather than assumed. */ + for (let attempt = 0; attempt < 2; attempt += 1) { + const result = await store.set( + ownerKey, + record.expected_session_key, + 'EX', + ownerTtlSeconds, + 'NX', + ); + if (result === 'OK') { + return 'ready'; + } + const raced = await store.get(ownerKey); + if (raced === record.expected_session_key) { + const refreshed = await refreshOwnerTtl(store, ownerKey, record, ownerTtlSeconds); + if (refreshed !== 'vanished') { + return refreshed; + } + continue; + } + if (raced !== null) { + return 'conflict'; + } + } + return 'unresolved'; +} + +/** + * Whether an apply would still have durable-record work to do. Keeps the + * dry run honest: a session whose cache key already matches can still need + * its owner record created or extended, and reporting it as fully in place + * invites operators to skip the apply. + */ +async function ownerRepairPending( + store: RecoveryStore, + record: RecoveryRecord, + ownerTtlSeconds: number, + existing: string | null, +): Promise { + if (existing === null) { + return true; + } + const remaining = await store.ttl(`session-owner:${record.session_id}`); + return remaining >= 0 && remaining < ownerTtlSeconds; +} + export async function recoverSessionCache( records: RecoveryRecord[], store: RecoveryStore, - options: Pick, + options: Pick & { ownerTtlSeconds?: number }, ): Promise { + const ownerTtlSeconds = Math.max( + options.ownerTtlSeconds ?? DEFAULT_SESSION_OWNER_TTL_SECONDS, + options.ttlSeconds, + ); const summary: RecoverySummary = { input: records.length, missing: 0, @@ -335,10 +498,59 @@ export async function recoverSessionCache( for (const record of records) { try { + /* Inspected first, and read-only: a durable owner that disagrees + * with the manifest settles the session before anything is written, + * including the cache key — restoring that would hand the manifest's + * claimant a day of access the service never granted it. */ + const inspection = await inspectOwnerRecord(store, record); + if (inspection.status === 'conflict') { + summary.conflicts += 1; + // eslint-disable-next-line no-console + console.error(`Conflict: ${record.session_id} has a durable owner record for a different owner`); + continue; + } + const redisKey = `session:${record.session_id}`; + + /* Deferred until the cache key agrees. The durable record outlives + * the cache key by design, so creating one for an owner the live + * cache key contradicts would outlast the evidence against it. + * Returns the bucket the record lands in when the durable half did + * not settle, or null to keep the cache-key bucket. */ + const settleOwnerRecord = async ( + restoredCacheKey: boolean, + ): Promise<'conflicts' | 'missing' | null> => { + if (!options.apply) { + const pending = await ownerRepairPending(store, record, ownerTtlSeconds, inspection.existing); + return pending ? 'missing' : null; + } + + const outcome = await ensureOwnerRecord(store, record, ownerTtlSeconds, inspection.existing); + if (outcome === 'ready') { + return null; + } + if (outcome === 'conflict') { + if (restoredCacheKey) { + /* Undo the grant just made: left in place it would authorize + * the manifest owner to read and delete for `ttlSeconds` while + * the durable record names somebody else. */ + await store.del(redisKey); + } + // eslint-disable-next-line no-console + console.error(`Conflict: ${record.session_id} durable owner record changed during recovery`); + return 'conflicts'; + } + /* The cache key stays: a session without a durable record is no + * worse off than before this run, and removing the grant would + * leave the operator with nothing at all. */ + // eslint-disable-next-line no-console + console.error(`Missing: ${record.session_id} durable owner record could not be created`); + return 'missing'; + }; + const current = await store.get(redisKey); if (current === record.expected_session_key) { - summary.matching += 1; + summary[await settleOwnerRecord(false) ?? 'matching'] += 1; continue; } if (current !== null) { @@ -361,13 +573,13 @@ export async function recoverSessionCache( 'NX', ); if (result === 'OK') { - summary.restored += 1; + summary[await settleOwnerRecord(true) ?? 'restored'] += 1; continue; } const racedValue = await store.get(redisKey); if (racedValue === record.expected_session_key) { - summary.matching += 1; + summary[await settleOwnerRecord(false) ?? 'matching'] += 1; } else if (racedValue === null) { summary.missing += 1; // eslint-disable-next-line no-console diff --git a/service/src/config.ts b/service/src/config.ts index f0be8ba4..8a8208f9 100644 --- a/service/src/config.ts +++ b/service/src/config.ts @@ -336,6 +336,16 @@ export const env = { FETCH_MAX_REQUESTS: Number(process.env.FETCH_MAX_REQUESTS) || 120, // 120 requests per minute // Redis Key Cache Config SESSION_CACHE_TTL: Number(process.env.SESSION_CACHE_TTL) || 86400, + /** TTL for the durable `session-owner:` record that backs + * deletion after `SESSION_CACHE_TTL` has lapsed (see + * `session-ownership.ts`). Sized to outlive a client's retention + * window — LibreChat sweeps expired files at 30 days by default, and a + * shorter value here reinstates the leak it exists to close. Clamped + * so it can never be tighter than the cache TTL. */ + SESSION_OWNER_TTL: Math.max( + Number(process.env.SESSION_OWNER_TTL) || 90 * 86400, + Number(process.env.SESSION_CACHE_TTL) || 86400, + ), /** Strict tenant isolation. When true, sessionKey resolution fails closed * (500) on requests whose auth context lacks `tenantId`, instead of * silently falling back to the `'legacy'` tenant prefix. Default OFF in diff --git a/service/src/middleware/auth.ts b/service/src/middleware/auth.ts index b402e346..f8e66835 100644 --- a/service/src/middleware/auth.ts +++ b/service/src/middleware/auth.ts @@ -4,6 +4,7 @@ import { connection } from '../queue'; import { isValidId } from '../utils'; import { env } from '../config'; import { resolveSessionKey, parseUploadSessionKeyInput, SessionKeyResolutionError } from '../session-key'; +import { authorizeSessionOwnership } from '../session-ownership'; import { LibreChatJwtAuthProvider, CodeApiJwtAuthError } from '../auth/librechat-jwt'; import { applyPrincipal, type CodeApiPrincipal } from '../auth/principal'; import { applyLocalPrincipal } from '../auth/local'; @@ -238,11 +239,36 @@ export const sessionAuth = async (req: AuthenticatedRequest, res: Response, next } throw err; } - const cachedSessionKey = await connection.get(`session:${session_id}`); - if (cachedSessionKey !== sessionKey) { - logger.error(`Unauthorized download: Cached session key: ${cachedSessionKey} | Expected session key: ${sessionKey} | Session ID: ${session_id} | File ID: ${fileId}`); + + /* A delete may fall back to the durable owner record once the session + * cache key has expired; reads keep the `SESSION_CACHE_TTL` window they + * have always had. Without the fallback an object is deletable only for + * the 24h following upload and is stranded in the bucket forever after + * that — see `session-ownership.ts`. */ + const isDelete = req.method === 'DELETE'; + let ownership: Awaited>; + try { + ownership = await authorizeSessionOwnership(connection, { + /* Narrowed by the `isValidId` guard above, which is not a type + * predicate. */ + session_id: session_id as string, + expectedSessionKey: sessionKey, + allowExpiredCache: isDelete, + }); + } catch (err) { + /* Express 4 does not forward a rejected async middleware, so an + * unavailable Redis — or an ACL that grants `session:*` but not + * `session-owner:*` — would hang the request instead of answering. */ + logger.error(`Session ownership lookup failed - Session ID: ${session_id} | File ID: ${fileId}`, err); + return res.status(500).json({ error: 'Internal server error' }); + } + if (!ownership.authorized) { + logger.error(`Unauthorized ${isDelete ? 'delete' : 'download'}: Cached session key: ${ownership.cachedSessionKey} | Expected session key: ${sessionKey} | Session ID: ${session_id} | File ID: ${fileId} | Reason: ${ownership.reason}`); return res.status(403).json({ error: 'Unauthorized' }); } + if (ownership.source === 'owner') { + logger.info(`Delete authorized from durable owner record - Session ID: ${session_id} | File ID: ${fileId}`); + } req.sessionKey = sessionKey; next(); diff --git a/service/src/rehydrate-session-cache.test.ts b/service/src/rehydrate-session-cache.test.ts index 22e391f8..4b9a0b5c 100644 --- a/service/src/rehydrate-session-cache.test.ts +++ b/service/src/rehydrate-session-cache.test.ts @@ -25,6 +25,7 @@ const SOURCE = { class MemoryStore implements RecoveryStore { readonly values = new Map(); + readonly ttls = new Map(); async get(key: string): Promise { return this.values.get(key) ?? null; @@ -34,15 +35,60 @@ class MemoryStore implements RecoveryStore { key: string, value: string, _expiryMode: 'EX', - _ttlSeconds: number, + ttlSeconds: number, _condition: 'NX', ): Promise<'OK' | null> { if (this.values.has(key)) { return null; } this.values.set(key, value); + this.ttls.set(key, ttlSeconds); return 'OK'; } + + async ttl(key: string): Promise { + if (!this.values.has(key)) { + return -2; + } + return this.ttls.get(key) ?? -1; + } + + async expire(key: string, ttlSeconds: number): Promise { + if (!this.values.has(key)) { + return 0; + } + this.ttls.set(key, ttlSeconds); + return 1; + } + + async del(key: string): Promise { + this.ttls.delete(key); + return this.values.delete(key) ? 1 : 0; + } +} + +/** Loses every `SET NX` on the durable owner key, optionally planting a + * different owner for the follow-up read to find. */ +class RacingOwnerStore extends MemoryStore { + constructor(private readonly ownerAfterRace: string | null) { + super(); + } + + async set( + key: string, + value: string, + expiryMode: 'EX', + ttlSeconds: number, + condition: 'NX', + ): Promise<'OK' | null> { + if (!key.startsWith('session-owner:')) { + return super.set(key, value, expiryMode, ttlSeconds, condition); + } + if (this.ownerAfterRace !== null) { + this.values.set(key, this.ownerAfterRace); + } + return null; + } } describe('rehydrate-session-cache', () => { @@ -156,6 +202,272 @@ describe('rehydrate-session-cache', () => { expect(() => parseOptions([], {})).toThrow('Recovery environment'); }); + it('defaults the owner TTL past the cache TTL and never below it', () => { + expect(parseOptions([], { + SESSION_RECOVERY_ENVIRONMENT: 'configured-env', + SESSION_RECOVERY_REGION: 'configured-region', + SESSION_RECOVERY_NAMESPACE: 'configured-namespace', + })).toMatchObject({ ttlSeconds: 86400, ownerTtlSeconds: 90 * 86400 }); + + expect(parseOptions(['--owner-ttl-seconds', '604800'], { + SESSION_RECOVERY_ENVIRONMENT: 'configured-env', + SESSION_RECOVERY_REGION: 'configured-region', + SESSION_RECOVERY_NAMESPACE: 'configured-namespace', + })).toMatchObject({ ownerTtlSeconds: 604800 }); + + /* A shorter owner TTL would re-strand the session it just recovered. */ + expect(parseOptions([ + '--ttl-seconds', '86400', + '--owner-ttl-seconds', '600', + ], { + SESSION_RECOVERY_ENVIRONMENT: 'configured-env', + SESSION_RECOVERY_REGION: 'configured-region', + SESSION_RECOVERY_NAMESPACE: 'configured-namespace', + })).toMatchObject({ ttlSeconds: 86400, ownerTtlSeconds: 86400 }); + }); + + it('restores the durable owner record alongside the cache key', async () => { + const store = new MemoryStore(); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toMatchObject({ restored: 1, conflicts: 0 }); + expect(store.values.get(`session:${SESSION_ID}`)).toBe(SESSION_KEY); + expect(store.ttls.get(`session:${SESSION_ID}`)).toBe(86400); + expect(store.values.get(`session-owner:${SESSION_ID}`)).toBe(SESSION_KEY); + expect(store.ttls.get(`session-owner:${SESSION_ID}`)).toBe(7776000); + }); + + it('backfills the owner record for a session whose cache key is still live', async () => { + const store = new MemoryStore(); + store.values.set(`session:${SESSION_ID}`, SESSION_KEY); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toMatchObject({ matching: 1, restored: 0, conflicts: 0 }); + expect(store.values.get(`session-owner:${SESSION_ID}`)).toBe(SESSION_KEY); + }); + + it('reports a conflicting owner record and restores nothing for that session', async () => { + const store = new MemoryStore(); + store.values.set(`session-owner:${SESSION_ID}`, 'tenant-id:user:someone-else'); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toEqual({ input: 1, missing: 0, restored: 0, matching: 0, conflicts: 1 }); + expect(store.values.get(`session-owner:${SESSION_ID}`)).toBe('tenant-id:user:someone-else'); + /* The manifest's claimant must not get a day of access the service + * never granted it. */ + expect(store.values.has(`session:${SESSION_ID}`)).toBe(false); + }); + + it('creates no durable owner record when the live cache key names another owner', async () => { + const store = new MemoryStore(); + store.values.set(`session:${SESSION_ID}`, 'tenant-id:user:someone-else'); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toEqual({ input: 1, missing: 0, restored: 0, matching: 0, conflicts: 1 }); + /* A durable record written here would outlive the cache key that + * contradicts it, and `sessionAuth` would then authorize the manifest + * owner to delete the real owner's files. */ + expect(store.values.has(`session-owner:${SESSION_ID}`)).toBe(false); + }); + + it('rolls back a restored cache key when the owner record turns out to be another owner', async () => { + const store = new RacingOwnerStore('tenant-id:user:someone-else'); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toEqual({ input: 1, missing: 0, restored: 0, matching: 0, conflicts: 1 }); + /* Left in place, the grant would authorize the manifest owner for a + * full ttlSeconds against a session the durable record says is not + * theirs. */ + expect(store.values.has(`session:${SESSION_ID}`)).toBe(false); + }); + + it('reports an owner record that cannot be created as missing', async () => { + const store = new RacingOwnerStore(null); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + /* `missing` during an apply is what drives the nonzero exit — the run + * must not look like a completed recovery. */ + expect(summary).toEqual({ input: 1, missing: 1, restored: 0, matching: 0, conflicts: 0 }); + expect(store.values.has(`session-owner:${SESSION_ID}`)).toBe(false); + /* The cache key stays: no durable record is where this session already + * was, and removing it would leave the operator with nothing. */ + expect(store.values.get(`session:${SESSION_ID}`)).toBe(SESSION_KEY); + }); + + it('reports pending owner work during a dry run', async () => { + const store = new MemoryStore(); + store.values.set(`session:${SESSION_ID}`, SESSION_KEY); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: false, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + /* Counting this as `matching` would tell an operator the session is + * fully in place and invite them to skip the apply that creates its + * durable record. */ + expect(summary).toEqual({ input: 1, missing: 1, restored: 0, matching: 0, conflicts: 0 }); + expect(store.values.has(`session-owner:${SESSION_ID}`)).toBe(false); + }); + + it('reports a short-lived owner record as pending work during a dry run', async () => { + const store = new MemoryStore(); + store.values.set(`session:${SESSION_ID}`, SESSION_KEY); + store.values.set(`session-owner:${SESSION_ID}`, SESSION_KEY); + store.ttls.set(`session-owner:${SESSION_ID}`, 600); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: false, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toEqual({ input: 1, missing: 1, restored: 0, matching: 0, conflicts: 0 }); + expect(store.ttls.get(`session-owner:${SESSION_ID}`)).toBe(600); + }); + + it('counts a fully in-place session as matching during a dry run', async () => { + const store = new MemoryStore(); + store.values.set(`session:${SESSION_ID}`, SESSION_KEY); + store.values.set(`session-owner:${SESSION_ID}`, SESSION_KEY); + store.ttls.set(`session-owner:${SESSION_ID}`, 7776000); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: false, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toEqual({ input: 1, missing: 0, restored: 0, matching: 1, conflicts: 0 }); + }); + + it('creates the owner record when it expires between inspection and refresh', async () => { + const store = new MemoryStore(); + store.values.set(`session:${SESSION_ID}`, SESSION_KEY); + store.values.set(`session-owner:${SESSION_ID}`, SESSION_KEY); + store.ttls.set(`session-owner:${SESSION_ID}`, 600); + const realTtl = store.ttl.bind(store); + let ttlReads = 0; + store.ttl = async (key: string): Promise => { + ttlReads += 1; + if (ttlReads === 1) { + /* Lapses in the window between the inspection read and the + * extension it was about to perform. */ + store.values.delete(`session-owner:${SESSION_ID}`); + store.ttls.delete(`session-owner:${SESSION_ID}`); + return -2; + } + return realTtl(key); + }; + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toEqual({ input: 1, missing: 0, restored: 0, matching: 1, conflicts: 0 }); + expect(store.values.get(`session-owner:${SESSION_ID}`)).toBe(SESSION_KEY); + expect(store.ttls.get(`session-owner:${SESSION_ID}`)).toBe(7776000); + }); + + it('reports an owner record replaced during the refresh as a conflict', async () => { + const store = new MemoryStore(); + store.values.set(`session:${SESSION_ID}`, SESSION_KEY); + store.values.set(`session-owner:${SESSION_ID}`, SESSION_KEY); + store.ttls.set(`session-owner:${SESSION_ID}`, 600); + const realExpire = store.expire.bind(store); + store.expire = async (key: string, ttlSeconds: number): Promise => { + const result = await realExpire(key, ttlSeconds); + store.values.set(`session-owner:${SESSION_ID}`, 'tenant-id:user:someone-else'); + return result; + }; + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + /* The extension itself is harmless — it prolongs a claim the service + * wrote — but the session must not be reported as recovered. */ + expect(summary).toEqual({ input: 1, missing: 0, restored: 0, matching: 0, conflicts: 1 }); + }); + + it('surfaces a conflicting owner record during a dry run', async () => { + const store = new MemoryStore(); + store.values.set(`session-owner:${SESSION_ID}`, 'tenant-id:user:someone-else'); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: false, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toEqual({ input: 1, missing: 0, restored: 0, matching: 0, conflicts: 1 }); + expect(store.values.size).toBe(1); + }); + + it('extends a matching owner record that would expire before the restored cache key', async () => { + const store = new MemoryStore(); + store.values.set(`session-owner:${SESSION_ID}`, SESSION_KEY); + store.ttls.set(`session-owner:${SESSION_ID}`, 600); + + const summary = await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(summary).toMatchObject({ restored: 1, conflicts: 0 }); + expect(store.ttls.get(`session-owner:${SESSION_ID}`)).toBe(7776000); + }); + + it('leaves a longer-lived owner record alone', async () => { + const store = new MemoryStore(); + store.values.set(`session-owner:${SESSION_ID}`, SESSION_KEY); + store.ttls.set(`session-owner:${SESSION_ID}`, 9999999); + + await recoverSessionCache( + [{ session_id: SESSION_ID, expected_session_key: SESSION_KEY }], + store, + { apply: true, ttlSeconds: 86400, ownerTtlSeconds: 7776000 }, + ); + + expect(store.ttls.get(`session-owner:${SESSION_ID}`)).toBe(9999999); + }); + it('does not write during a dry run', async () => { const store = new MemoryStore(); const summary = await recoverSessionCache( @@ -189,9 +501,12 @@ describe('rehydrate-session-cache', () => { it('preserves partial counts when a Redis operation fails', async () => { let reads = 0; const store: RecoveryStore = { + /* Three reads to settle the first record: the durable owner, the + * cache key, then the owner re-read that confirms the refresh. The + * second record fails on its first read. */ async get(): Promise { reads += 1; - if (reads === 1) { + if (reads <= 3) { return SESSION_KEY; } throw new Error('Redis unavailable'); @@ -199,6 +514,15 @@ describe('rehydrate-session-cache', () => { async set(): Promise<'OK' | null> { throw new Error('unexpected set'); }, + async ttl(): Promise { + return 7776000; + }, + async expire(): Promise { + throw new Error('unexpected expire'); + }, + async del(): Promise { + throw new Error('unexpected del'); + }, }; try { @@ -231,6 +555,15 @@ describe('rehydrate-session-cache', () => { async set(): Promise { return null; }, + async ttl(): Promise { + return -2; + }, + async expire(): Promise { + return 0; + }, + async del(): Promise { + return 0; + }, }; const summary = await recoverSessionCache( diff --git a/service/src/service/programmatic-router.ts b/service/src/service/programmatic-router.ts index baa350e6..4536270b 100644 --- a/service/src/service/programmatic-router.ts +++ b/service/src/service/programmatic-router.ts @@ -40,6 +40,7 @@ import { } from '../sandbox-egress'; import { findUnregisteredToolCall } from '../tool-scope'; import { summarizeRequestedFiles } from '../execution-log'; +import { clearSessionOwnership, recordSessionOwnership } from '../session-ownership'; import { FileRefAuthorizationError, authorizeRequestedFiles } from './file-authorization'; import { buildReplayExecutionState, @@ -562,7 +563,7 @@ async function handleReplayInitial( code.includes('import matplotlib') || code.includes('import seaborn') ); - await connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL); + await recordSessionOwnership(connection, session_id, sessionKey); const state = buildReplayExecutionState({ executionId: execution_id, @@ -605,7 +606,7 @@ async function handleReplayInitial( bytes: err.bytes, cap: err.cap, }); - await connection.del(`session:${session_id}`).catch(() => {}); + await clearSessionOwnership(connection, session_id).catch(() => {}); ptcReplayStateOversize.inc(); res.status(413).json({ error: `Request too large: serialized execution state is ${err.bytes} bytes (max ${err.cap}). Reduce the size of "code", "tools", or "files".`, @@ -1315,7 +1316,12 @@ async function handleBlocking( const execution_id = nanoid(); const identity = getExecutionIdentity(req, userId); - connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL); + /* Awaited: a partial registration (cache key written, durable record + * refused — a Redis ACL scoped to `session:*` would do it) would let the + * job write files that become undeletable once `SESSION_CACHE_TTL` + * lapses. The caller turns a rejection into a 500 before anything is + * enqueued. */ + await recordSessionOwnership(connection, session_id, sessionKey); const executionState: ExecutionState = { execution_id, diff --git a/service/src/service/router.ts b/service/src/service/router.ts index 971bb7fd..f1a840be 100644 --- a/service/src/service/router.ts +++ b/service/src/service/router.ts @@ -24,6 +24,7 @@ import { captureTraceCarrier, withSpan } from '../telemetry'; import { Jobs, Languages } from '../enum'; import { FileRefAuthorizationError, authorizeRequestedFiles } from './file-authorization'; import { createUploadSessionRegistrar } from './upload-session'; +import { recordSessionOwnership } from '../session-ownership'; import { prepareSandboxJobSecurity } from '../sandbox-egress'; import { BridgeWorkerSelectionError, @@ -219,7 +220,16 @@ router.post('/exec', executionLimiter, async (req: t.AuthenticatedRequest, res) * sandbox invocation." */ const session_id = nanoid(); const execution_id = nanoid(); - await connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL); + /* Guarded: registration runs before the route's `try`, and Express 4 + * does not forward a rejected async handler to the error middleware — + * an unavailable Redis, or an ACL that permits `session:*` but not + * `session-owner:*`, would hang the request instead of answering. */ + try { + await recordSessionOwnership(connection, session_id, sessionKey); + } catch (error) { + logger.error(`[${INSTANCE_ID}] Error registering session ownership - Session ID: ${session_id}:`, error); + return res.status(500).json({ error: 'Internal server error' }); + } try { if (!isSyntheticRequest) { @@ -478,7 +488,7 @@ router.post('/upload', uploadLimiter, async (req: t.AuthenticatedRequest, res: R if (readOnly) { putHeaders['X-Read-Only'] = 'true'; } - connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL) + recordSessionOwnership(connection, session_id, sessionKey) .then(() => { logger.info(`[${INSTANCE_ID}] Upload: Session ID: ${session_id} | User ID: ${userId} | Session key: ${sessionKey}`); return axios.put( @@ -600,7 +610,7 @@ router.post('/upload/batch', uploadLimiter, async (req: t.AuthenticatedRequest, const ensureSessionRegistered = createUploadSessionRegistrar((sessionKey) => { logger.info(`[${INSTANCE_ID}] Batch upload: Session ID: ${session_id} | User ID: ${userId} | Session key: ${sessionKey}`); - return connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL); + return recordSessionOwnership(connection, session_id, sessionKey); }); const planFileSize = planLimits[req.planId ?? '']?.max_file_size ?? planLimits.default.max_file_size; @@ -904,7 +914,15 @@ router.get('/sessions/:session_id/objects/:fileId', fetchLimiter, sessionAuth, a } }); -router.delete('/files/:session_id/:fileId', fetchLimiter, sessionAuth, async (req: t.AuthenticatedRequest, res: Response) => { +/** + * Remove a session object. + * + * Mounted on two paths (see the registrations below); both are gated by + * `sessionAuth`, so the caller has to own the `(session_id, entity_id)` + * pair the object was stored under, and both proxy the same file-server + * route. + */ +const deleteSessionObject = async (req: t.AuthenticatedRequest, res: Response) => { const { session_id, fileId } = req.params; try { @@ -917,12 +935,46 @@ router.delete('/files/:session_id/:fileId', fetchLimiter, sessionAuth, async (re logger.info(`[${INSTANCE_ID}] File deleted: Session ID: ${session_id} | File ID: ${fileId}`); return res.status(200).json(response.data); } catch (error) { + /* The file-server answers 404 when the object is already gone. Pass + * that through instead of collapsing it into a 500: a client sweeping + * expired files can retire the reference on 404, whereas a 500 reads + * as retryable and has it re-issuing the same DELETE for an object + * that no longer exists on every subsequent pass. */ + if (axios.isAxiosError(error) && error.response?.status === 404) { + /* Best effort: this runs inside the catch block, where a rejection + * has no handler above it — Express 4 does not forward async + * rejections, so it would hang the request instead of answering. + * The key expires on its own, and the object is already gone. */ + await connection.del(`upload:${req.sessionKey}${session_id}${fileId}`).catch((err: unknown) => { + logger.warn(`[${INSTANCE_ID}] Failed to clear upload key for absent file - Session ID: ${session_id} | File ID: ${fileId}:`, err); + }); + logger.info(`[${INSTANCE_ID}] File already absent: Session ID: ${session_id} | File ID: ${fileId}`); + return res.status(404).json({ error: 'File not found' }); + } const errorDetails = getAxiosErrorDetails(error); logger.error(`[${INSTANCE_ID}] Error deleting file - Session ID: ${session_id} | File ID: ${fileId}:`, errorDetails); return res.status(500).json({ error: 'Error deleting file', }); } -}); +}; + +router.delete('/files/:session_id/:fileId', fetchLimiter, sessionAuth, deleteSessionObject); + +/** + * Alias of the route above, on the path LibreChat's `deleteCodeEnvFile` + * targets — the file-server's own DELETE path, which is not itself + * exposed on `/v1`. + * + * Until LibreChat v0.8.6 this was the only path the client tried, and the + * 404 from an unmounted method was indistinguishable from "the object is + * already gone": every deletion silently failed and objects accumulated + * with nothing to alert on. Newer clients fall back to `/files/...`, but + * mounting the alias costs nothing and makes deletion work for + * deployments still running an older client. + * + * GET on this same path is the metadata proxy above. + */ +router.delete('/sessions/:session_id/objects/:fileId', fetchLimiter, sessionAuth, deleteSessionObject); export default router; diff --git a/service/src/session-ownership.test.ts b/service/src/session-ownership.test.ts new file mode 100644 index 00000000..bc9e2b74 --- /dev/null +++ b/service/src/session-ownership.test.ts @@ -0,0 +1,165 @@ +import { describe, expect, test } from 'bun:test'; + +import { + authorizeSessionOwnership, + clearSessionOwnership, + recordSessionOwnership, + sessionCacheKey, + sessionOwnerKey, + type SessionOwnershipStore, +} from './session-ownership'; + +interface Written { + value: string; + ttl: number; +} + +function createStore(seed: Record = {}) { + const values = new Map(Object.entries(seed)); + const writes: Record = {}; + const store: SessionOwnershipStore = { + get: async (key) => values.get(key) ?? null, + set: async (key, value, _expiryMode, ttlSeconds) => { + values.set(key, value); + writes[key] = { value, ttl: ttlSeconds }; + return 'OK'; + }, + del: async (...keys) => { + let removed = 0; + for (const key of keys) { + if (values.delete(key)) removed += 1; + } + return removed; + }, + }; + return { store, writes, values }; +} + +const SESSION_ID = 'session-1'; +const OWNER = 'tenant-1:user:user-1'; + +describe('recordSessionOwnership', () => { + test('writes the cache key and the durable owner record together', async () => { + const { store, writes } = createStore(); + + await recordSessionOwnership(store, SESSION_ID, OWNER, { cacheTtl: 100, ownerTtl: 9000 }); + + expect(writes[sessionCacheKey(SESSION_ID)]).toEqual({ value: OWNER, ttl: 100 }); + expect(writes[sessionOwnerKey(SESSION_ID)]).toEqual({ value: OWNER, ttl: 9000 }); + }); +}); + +describe('clearSessionOwnership', () => { + test('rolls back both records', async () => { + const { store, values } = createStore(); + await recordSessionOwnership(store, SESSION_ID, OWNER, { cacheTtl: 100, ownerTtl: 9000 }); + + await clearSessionOwnership(store, SESSION_ID); + + expect(values.has(sessionCacheKey(SESSION_ID))).toBe(false); + expect(values.has(sessionOwnerKey(SESSION_ID))).toBe(false); + }); +}); + +describe('authorizeSessionOwnership', () => { + test('authorizes from the cache key while it is live', async () => { + const { store } = createStore({ [sessionCacheKey(SESSION_ID)]: OWNER }); + + const result = await authorizeSessionOwnership(store, { + session_id: SESSION_ID, + expectedSessionKey: OWNER, + allowExpiredCache: false, + }); + + expect(result).toEqual({ authorized: true, source: 'session' }); + }); + + test('denies a read once the cache key has expired, even with an owner record', async () => { + const { store } = createStore({ [sessionOwnerKey(SESSION_ID)]: OWNER }); + + const result = await authorizeSessionOwnership(store, { + session_id: SESSION_ID, + expectedSessionKey: OWNER, + allowExpiredCache: false, + }); + + expect(result).toEqual({ authorized: false, reason: 'expired', cachedSessionKey: null }); + }); + + test('authorizes a delete from the owner record once the cache key has expired', async () => { + const { store } = createStore({ [sessionOwnerKey(SESSION_ID)]: OWNER }); + + const result = await authorizeSessionOwnership(store, { + session_id: SESSION_ID, + expectedSessionKey: OWNER, + allowExpiredCache: true, + }); + + expect(result).toEqual({ authorized: true, source: 'owner' }); + }); + + test('a live cache key naming another owner is authoritative and blocks the fallback', async () => { + const { store } = createStore({ + [sessionCacheKey(SESSION_ID)]: 'tenant-1:user:someone-else', + /* Stale owner record that would otherwise match — a re-registered + * session must not be deletable by its previous owner. */ + [sessionOwnerKey(SESSION_ID)]: OWNER, + }); + + const result = await authorizeSessionOwnership(store, { + session_id: SESSION_ID, + expectedSessionKey: OWNER, + allowExpiredCache: true, + }); + + expect(result).toEqual({ + authorized: false, + reason: 'mismatch', + cachedSessionKey: 'tenant-1:user:someone-else', + }); + }); + + test('denies a delete when the owner record names someone else', async () => { + const { store } = createStore({ [sessionOwnerKey(SESSION_ID)]: 'tenant-2:user:user-2' }); + + const result = await authorizeSessionOwnership(store, { + session_id: SESSION_ID, + expectedSessionKey: OWNER, + allowExpiredCache: true, + }); + + expect(result).toEqual({ + authorized: false, + reason: 'mismatch', + cachedSessionKey: 'tenant-2:user:user-2', + }); + }); + + test('reports sessions that predate the owner record as unknown', async () => { + const { store } = createStore(); + + const result = await authorizeSessionOwnership(store, { + session_id: SESSION_ID, + expectedSessionKey: OWNER, + allowExpiredCache: true, + }); + + expect(result).toEqual({ authorized: false, reason: 'unknown', cachedSessionKey: null }); + }); + + test('a session registered through recordSessionOwnership stays deletable past the cache TTL', async () => { + const { store, values } = createStore(); + await recordSessionOwnership(store, SESSION_ID, OWNER, { cacheTtl: 1, ownerTtl: 9000 }); + + /* Simulate the cache key aging out while the owner record lives on. */ + values.delete(sessionCacheKey(SESSION_ID)); + + expect( + await authorizeSessionOwnership(store, { + session_id: SESSION_ID, + expectedSessionKey: OWNER, + allowExpiredCache: true, + }), + ).toEqual({ authorized: true, source: 'owner' }); + }); +}); diff --git a/service/src/session-ownership.ts b/service/src/session-ownership.ts new file mode 100644 index 00000000..bf4ba6dd --- /dev/null +++ b/service/src/session-ownership.ts @@ -0,0 +1,126 @@ +import { env } from './config'; + +/** + * Ownership of a session's stored objects is recorded twice. + * + * `session:` is the hot path: `sessionAuth` compares it on + * every download, metadata fetch and execution input, and its + * `SESSION_CACHE_TTL` (24h by default) is deliberately short — it bounds + * how long a sandbox invocation's outputs stay reachable. + * + * That bound is wrong for deletion. A file is deletable only while + * someone can still prove they own it, so with one key the window in + * which an object can be removed closes a day after upload and the + * object is stranded for the life of the deployment: unreadable, + * unusable as an execution input, and undeletable through every route. + * Clients sweeping their own retention window are typically far outside + * 24h when they get there (LibreChat's default retention is 30 days), so + * in practice every swept object failed to delete and the bucket only + * ever grew. + * + * `session-owner:` is the durable half: same value, TTL + * `SESSION_OWNER_TTL`, consulted only when the cache key has expired and + * only for deletes. Read access keeps the original 24h bound. + */ + +/** The subset of the Redis client these helpers need — narrow enough to + * fake in tests without standing up a connection. */ +export interface SessionOwnershipStore { + get(key: string): Promise; + set(key: string, value: string, expiryMode: 'EX', ttlSeconds: number): Promise; + del(...keys: string[]): Promise; +} + +export const sessionCacheKey = (session_id: string): string => `session:${session_id}`; +export const sessionOwnerKey = (session_id: string): string => `session-owner:${session_id}`; + +export interface SessionOwnershipTtls { + cacheTtl?: number; + ownerTtl?: number; +} + +/** + * Register `sessionKey` as the owner of `session_id`, writing both the + * cache key and the durable owner record. Replaces the bare + * `connection.set('session:…')` at every site that opens a session, so + * the two can never drift apart. + */ +export function recordSessionOwnership( + store: SessionOwnershipStore, + session_id: string, + sessionKey: string, + ttls: SessionOwnershipTtls = {}, +): Promise { + const cacheTtl = ttls.cacheTtl ?? env.SESSION_CACHE_TTL; + const ownerTtl = ttls.ownerTtl ?? env.SESSION_OWNER_TTL; + return Promise.all([ + store.set(sessionCacheKey(session_id), sessionKey, 'EX', cacheTtl), + store.set(sessionOwnerKey(session_id), sessionKey, 'EX', ownerTtl), + ]); +} + +/** + * Roll back a registration, dropping both records. Used where a request + * is rejected after opening a session — leaving the durable half behind + * would keep an owner record alive for `SESSION_OWNER_TTL` on a session + * that never stored anything. + */ +export function clearSessionOwnership( + store: SessionOwnershipStore, + session_id: string, +): Promise { + return store.del(sessionCacheKey(session_id), sessionOwnerKey(session_id)); +} + +export type SessionOwnershipSource = 'session' | 'owner'; + +/** `expired`: nothing live, and the durable record was not consulted or + * had also lapsed. `unknown`: the session predates the owner record, so + * ownership can no longer be established (recoverable with + * `scripts/rehydrate-session-cache.ts`). `mismatch`: a recorded owner + * exists and it is somebody else. */ +export type SessionOwnershipDenial = 'expired' | 'unknown' | 'mismatch'; + +export type SessionOwnershipResult = + | { authorized: true; source: SessionOwnershipSource } + | { authorized: false; reason: SessionOwnershipDenial; cachedSessionKey: string | null }; + +/** + * Decide whether `expectedSessionKey` owns `session_id`. + * + * A live cache key is always authoritative — when one exists and names a + * different owner the answer is no, and the durable record is not + * consulted. The fallback only covers the case where the cache key is + * simply gone. + */ +export async function authorizeSessionOwnership( + store: SessionOwnershipStore, + args: { + session_id: string; + expectedSessionKey: string; + /** Enable the durable fallback. Deletes pass true; reads keep the + * `SESSION_CACHE_TTL` window they have always had. */ + allowExpiredCache: boolean; + }, +): Promise { + const { session_id, expectedSessionKey, allowExpiredCache } = args; + const cachedSessionKey = await store.get(sessionCacheKey(session_id)); + if (cachedSessionKey === expectedSessionKey) { + return { authorized: true, source: 'session' }; + } + if (cachedSessionKey !== null) { + return { authorized: false, reason: 'mismatch', cachedSessionKey }; + } + if (!allowExpiredCache) { + return { authorized: false, reason: 'expired', cachedSessionKey }; + } + + const recordedOwner = await store.get(sessionOwnerKey(session_id)); + if (recordedOwner === expectedSessionKey) { + return { authorized: true, source: 'owner' }; + } + if (recordedOwner === null) { + return { authorized: false, reason: 'unknown', cachedSessionKey }; + } + return { authorized: false, reason: 'mismatch', cachedSessionKey: recordedOwner }; +}