fix(plugins): reject path traversal in manifest identity before the install rm

middleware/src/plugins/manifestLoader.ts

@@ -162,6 +162,21 @@ async function loadManifestV1Entries(
   return entries;
 }
 
+/**
+ * `identity.id` — an npm package name, optionally scoped. Deliberately
+ * narrower than npm's own rules (lowercase only, no URL-escapes, exactly one
+ * `/` and only after a `@scope`): every legitimate omadia plugin id is either
+ * scoped (`@omadia/plugin-office`) or a reverse-FQDN (`de.byte5.agent.foo`),
+ * and both fit. Matches the Builder's `AgentIdSchema` in builder/agentSpec.ts
+ * modulo the `_`/leading-digit tolerance npm allows.
+ */
+const PLUGIN_ID_PATTERN = /^(?:@[a-z0-9][a-z0-9._-]*\/)?[a-z0-9][a-z0-9._-]*$/;
+/** npm's own package-name cap, including the scope. */
+const PLUGIN_ID_MAX_LENGTH = 214;
+/** `identity.version` — semver `x.y.z` with optional prerelease/build. */
+const PLUGIN_VERSION_PATTERN =
+  /^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/;
+
 export function adaptManifestV1(doc: Record<string, unknown>): Plugin | null {
   if (doc['schema_version'] !== '1') return null;
 
@@ -173,6 +188,27 @@ export function adaptManifestV1(doc: Record<string, unknown>): Plugin | null {
   const version = asString(identity['version']);
   if (!id || !name || !version) return null;
 
+  // `id` and `version` are path segments downstream (the install directory is
+  // `<packagesDir>/<id>/<version>`), so they are security fields, not cosmetic
+  // metadata: an id of `..` plus a version of `migrations` would resolve to a
+  // sibling of the packages root and get `fs.rm`'d before the rename. Both are
+  // therefore REJECTED rather than sanitised — a manifest that cannot state
+  // its own identity in the documented charset is not a manifest we install.
+  // This is a hard reject (`null`), not the graceful degradation the optional
+  // blocks below use; the upload path turns it into `package.manifest_invalid`.
+  if (id.length > PLUGIN_ID_MAX_LENGTH || !PLUGIN_ID_PATTERN.test(id)) {
+    console.warn(
+      `[catalog] manifest rejected: identity.id ${JSON.stringify(id)} is not a lowercase npm-style package name (optionally @scoped, max ${PLUGIN_ID_MAX_LENGTH} chars).`,
+    );
+    return null;
+  }
+  if (!PLUGIN_VERSION_PATTERN.test(version)) {
+    console.warn(
+      `[catalog] manifest rejected: plugin '${id}' has identity.version ${JSON.stringify(version)}, which is not semver (x.y.z[-prerelease][+build]).`,
+    );
+    return null;
+  }
+
   const compat = asRecord(doc['compat']);
   const setup = asRecord(doc['setup']);
   const permissions = asRecord(doc['permissions']);

middleware/src/plugins/packageUploadService.ts

@@ -294,11 +294,20 @@ export class PackageUploadService {
       }
 
       // --- 10. Atomic rename into the final directory ----------------------
-      const finalDir = path.join(
+      // Containment is re-asserted here, independently of the charset gate in
+      // the manifest loader: the next two statements are an `fs.rm -rf` and a
+      // rename, so this is the last place where a traversing id/version can
+      // still be stopped, and it must not depend on a single upstream check
+      // staying correct.
+      const contained = resolveContainedPackageDir(
         this.deps.packagesDir,
         plugin.id,
         plugin.version,
       );
+      if (!contained.ok) {
+        return fail('package.path_traversal', contained.message);
+      }
+      const finalDir = contained.dir;
       await fs.mkdir(path.dirname(finalDir), { recursive: true });
       // If leftovers from an aborted installation are lying around → remove.
       await fs.rm(finalDir, { recursive: true, force: true });
@@ -422,6 +431,51 @@ export class PackageUploadService {
 // helpers
 // ---------------------------------------------------------------------------
 
+/**
+ * Entries that live DIRECTLY under the packages root and are not plugin
+ * directories: the host-node_modules symlink written by
+ * {@link ensureHostNodeModulesLink} and the store index. A plugin id equal to
+ * one of them stays inside the root as a string but would either follow the
+ * symlink out of the packages tree (into the host's real `node_modules`) or
+ * collide with the index file. `.staging` needs no entry — a leading dot is
+ * already outside the id charset.
+ */
+const RESERVED_ROOT_ENTRIES: ReadonlySet<string> = new Set([
+  'node_modules',
+  'index.json',
+]);
+
+/**
+ * Resolves `<packagesDir>/<id>/<version>` and proves it stays under the
+ * packages root. Defence in depth for the manifest loader's charset gate: the
+ * caller is about to `fs.rm -rf` this path, so containment is verified here
+ * from the raw values rather than assumed. Exported so the escape branch can
+ * be exercised directly — through `ingest` the loader's charset gate rejects
+ * a traversing id first, which is the point of having two layers.
+ */
+export function resolveContainedPackageDir(
+  packagesDir: string,
+  pluginId: string,
+  version: string,
+): { ok: true; dir: string } | { ok: false; message: string } {
+  const root = path.resolve(packagesDir);
+  const dir = path.resolve(path.join(root, pluginId, version));
+  if (!dir.startsWith(root + path.sep)) {
+    return {
+      ok: false,
+      message: `manifest.identity id/version resolve outside the packages directory (id=${JSON.stringify(pluginId)}, version=${JSON.stringify(version)}).`,
+    };
+  }
+  const firstSegment = path.relative(root, dir).split(path.sep)[0];
+  if (firstSegment !== undefined && RESERVED_ROOT_ENTRIES.has(firstSegment)) {
+    return {
+      ok: false,
+      message: `manifest.identity.id ${JSON.stringify(pluginId)} collides with the reserved packages-root entry '${firstSegment}'.`,
+    };
+  }
+  return { ok: true, dir };
+}
+
 async function resolvePackageRoot(stagingRoot: string): Promise<string | null> {
   if (await fileExists(path.join(stagingRoot, 'manifest.yaml'))) {
     return stagingRoot;