reporting-governance: harden artifact root boundary checks

docs/architecture/agent-reporting-governance-plugin.md

@@ -227,6 +227,7 @@ Architectural meaning:
 - storage layer owns loading/package-artifact interpretation
 - runtime binding can be derived from the artifact rather than hardcoded entirely in docs
 - tests prove the artifact resolves into concrete script and runtime-artifact paths
+- `artifact_roots` enforcement is now two-layered for this slice: lexical boundary rejection plus realpath-level symlink escape rejection
 
 This is intentionally still a **minimal verifiable slice**, not the full deployment system.
 It proves the package boundary can own profile artifacts and bind them into runtime execution inputs.

plugins/reporting-governance/README.md

@@ -158,8 +158,9 @@ What this slice does:
 2. loader resolves that artifact from package-local path
 3. validator fail-closes minimal boundary drift on `kind`, `apiVersion`, `spec.bindings.entrypoint`, `scripts`, `artifact_roots`, and `spec.package.pluginVersion`
 4. binding contract translates profile-declared script/artifact roots into concrete repo/runtime paths
-5. adapter runtime binding can be instantiated from that contract in tests
+5. validator rejects `artifact_roots` absolute paths, lexical escapes, and symlink escapes that resolve outside repo realpath boundary
-6. orchestrator adapter can now bootstrap from package profile artifact input directly
+6. adapter runtime binding can be instantiated from that contract in tests
+7. orchestrator adapter can now bootstrap from package profile artifact input directly
 
 What this slice does **not** claim yet:
 

plugins/reporting-governance/src/storage/profile-artifact.mjs

@@ -24,7 +24,36 @@ function assertObjectRecord(value, label) {
   return value;
 }
 
-function assertRelativePathWithinRoot(relativePath, label, { root }) {
+function assertPathWithinRealRoot(candidatePath, label, { root, allowMissingLeaf = false }) {
+  const realRoot = fs.realpathSync(root);
+  let realCandidate;
+
+  if (allowMissingLeaf && !fs.existsSync(candidatePath)) {
+    let cursor = path.dirname(candidatePath);
+    while (cursor !== root && !fs.existsSync(cursor)) {
+      cursor = path.dirname(cursor);
+    }
+
+    if (!fs.existsSync(cursor)) {
+      realCandidate = realRoot;
+    } else {
+      realCandidate = path.resolve(fs.realpathSync(cursor), path.relative(cursor, candidatePath));
+    }
+  } else {
+    realCandidate = fs.realpathSync(candidatePath);
+  }
+
+  const relativeToRealRoot = path.relative(realRoot, realCandidate);
+  if (
+    relativeToRealRoot === '..'
+    || relativeToRealRoot.startsWith(`..${path.sep}`)
+    || path.isAbsolute(relativeToRealRoot)
+  ) {
+    throw new Error(`${label} must stay within repo root: symlink resolution escapes realpath boundary`);
+  }
+}
+
+function assertRelativePathWithinRoot(relativePath, label, { root, allowMissingLeaf = false }) {
   const normalizedPath = assertNonEmptyString(relativePath, label);
   if (path.isAbsolute(normalizedPath)) {
     throw new Error(`${label} must stay within repo root: absolute paths are not allowed`);
@@ -40,6 +69,7 @@ function assertRelativePathWithinRoot(relativePath, label, { root }) {
     throw new Error(`${label} must stay within repo root: path escapes root boundary`);
   }
 
+  assertPathWithinRealRoot(resolvedPath, label, { root, allowMissingLeaf });
   return normalizedPath;
 }
 
@@ -70,7 +100,7 @@ export function validateDeploymentProfileArtifact(artifact, { repoRootOverride }
     assertRelativePathWithinRoot(relativePath, `deployment profile artifact spec.bindings.scripts.${key}`, { root });
   }
   for (const [key, relativePath] of Object.entries(artifactRoots)) {
-    assertRelativePathWithinRoot(relativePath, `deployment profile artifact spec.bindings.artifact_roots.${key}`, { root });
+    assertRelativePathWithinRoot(relativePath, `deployment profile artifact spec.bindings.artifact_roots.${key}`, { root, allowMissingLeaf: true });
   }
 
   return artifact;

plugins/reporting-governance/test/profile-artifact.test.mjs

@@ -1,6 +1,7 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
 import fs from 'node:fs';
+import os from 'node:os';
 import path from 'node:path';
 
 import {
@@ -13,6 +14,23 @@ import { createRuntimeBinding } from '../src/adapters/index.mjs';
 const packageRoot = path.resolve(import.meta.dirname, '..');
 const repoRoot = path.resolve(packageRoot, '..', '..');
 
+function createArtifact(overrides = {}) {
+  return {
+    kind: 'DeploymentProfileArtifact',
+    apiVersion: 'reporting-governance/v1alpha1',
+    spec: {
+      package: { pluginVersion: '0.1.0-mainline' },
+      bindings: {
+        runtime: 'openclaw',
+        entrypoint: 'scripts/watchdog_auto_notify_orchestrator.mjs',
+        scripts: { watchdog: 'scripts/long_task_watchdog.mjs' },
+        artifact_roots: { queueItems: 'state/operator-notify-queue' },
+      },
+    },
+    ...overrides,
+  };
+}
+
 test('deployment profile artifact loads from package profiles and preserves compatibility envelope metadata', () => {
   const { artifactPath, artifact } = loadDeploymentProfileArtifact({ profileId: 'strict-manager-mode' });
 
@@ -57,9 +75,7 @@ test('deployment profile artifact validation fails closed on boundary drift', ()
   );
 
   assert.throws(
-    () => validateDeploymentProfileArtifact({
+    () => validateDeploymentProfileArtifact(createArtifact({
-      kind: 'DeploymentProfileArtifact',
-      apiVersion: 'reporting-governance/v1alpha1',
       spec: {
         package: { pluginVersion: '0.1.0-mainline' },
         bindings: {
@@ -68,14 +84,12 @@ test('deployment profile artifact validation fails closed on boundary drift', ()
           artifact_roots: { queueItems: 'state/operator-notify-queue' },
         },
       },
-    }),
+    })),
     /spec\.bindings\.entrypoint must be a non-empty string/
   );
 
   assert.throws(
-    () => validateDeploymentProfileArtifact({
+    () => validateDeploymentProfileArtifact(createArtifact({
-      kind: 'DeploymentProfileArtifact',
-      apiVersion: 'reporting-governance/v1alpha1',
       spec: {
         package: { pluginVersion: '' },
         bindings: {
@@ -84,14 +98,12 @@ test('deployment profile artifact validation fails closed on boundary drift', ()
           artifact_roots: { queueItems: 'state/operator-notify-queue' },
         },
       },
-    }),
+    })),
     /spec\.package\.pluginVersion must be a non-empty string/
   );
 
   assert.throws(
-    () => validateDeploymentProfileArtifact({
+    () => validateDeploymentProfileArtifact(createArtifact({
-      kind: 'DeploymentProfileArtifact',
-      apiVersion: 'reporting-governance/v1alpha1',
       spec: {
         package: { pluginVersion: '0.1.0-mainline' },
         bindings: {
@@ -100,16 +112,14 @@ test('deployment profile artifact validation fails closed on boundary drift', ()
           artifact_roots: { queueItems: 'state/operator-notify-queue' },
         },
       },
-    }),
+    })),
     /spec\.bindings\.scripts must be an object record/
   );
 });
 
 test('deployment profile artifact validation rejects absolute binding paths', () => {
   assert.throws(
-    () => validateDeploymentProfileArtifact({
+    () => validateDeploymentProfileArtifact(createArtifact({
-      kind: 'DeploymentProfileArtifact',
-      apiVersion: 'reporting-governance/v1alpha1',
       spec: {
         package: { pluginVersion: '0.1.0-mainline' },
         bindings: {
@@ -118,14 +128,12 @@ test('deployment profile artifact validation rejects absolute binding paths', ()
           artifact_roots: { queueItems: 'state/operator-notify-queue' },
         },
       },
-    }),
+    })),
     /spec\.bindings\.entrypoint must stay within repo root: absolute paths are not allowed/
   );
 
   assert.throws(
-    () => validateDeploymentProfileArtifact({
+    () => validateDeploymentProfileArtifact(createArtifact({
-      kind: 'DeploymentProfileArtifact',
-      apiVersion: 'reporting-governance/v1alpha1',
       spec: {
         package: { pluginVersion: '0.1.0-mainline' },
         bindings: {
@@ -134,16 +142,28 @@ test('deployment profile artifact validation rejects absolute binding paths', ()
           artifact_roots: { queueItems: 'state/operator-notify-queue' },
         },
       },
-    }),
+    })),
     /spec\.bindings\.scripts\.watchdog must stay within repo root: absolute paths are not allowed/
   );
+
+  assert.throws(
+    () => validateDeploymentProfileArtifact(createArtifact({
+      spec: {
+        package: { pluginVersion: '0.1.0-mainline' },
+        bindings: {
+          entrypoint: 'scripts/watchdog_auto_notify_orchestrator.mjs',
+          scripts: { watchdog: 'scripts/long_task_watchdog.mjs' },
+          artifact_roots: { queueItems: '/abs/path' },
+        },
+      },
+    })),
+    /spec\.bindings\.artifact_roots\.queueItems must stay within repo root: absolute paths are not allowed/
+  );
 });
 
 test('deployment profile artifact validation rejects escape paths after resolution', () => {
   assert.throws(
-    () => validateDeploymentProfileArtifact({
+    () => validateDeploymentProfileArtifact(createArtifact({
-      kind: 'DeploymentProfileArtifact',
-      apiVersion: 'reporting-governance/v1alpha1',
       spec: {
         package: { pluginVersion: '0.1.0-mainline' },
         bindings: {
@@ -152,15 +172,13 @@ test('deployment profile artifact validation rejects escape paths after resoluti
           artifact_roots: { queueItems: 'state/operator-notify-queue' },
         },
       },
-    }),
+    })),
     /spec\.bindings\.entrypoint must stay within repo root: path escapes root boundary/
   );
 
   assert.throws(
     () => createDeploymentBindingContract({
-      artifact: {
+      artifact: createArtifact({
-        kind: 'DeploymentProfileArtifact',
-        apiVersion: 'reporting-governance/v1alpha1',
         spec: {
           package: { pluginVersion: '0.1.0-mainline' },
           bindings: {
@@ -169,18 +187,30 @@ test('deployment profile artifact validation rejects escape paths after resoluti
             artifact_roots: { queueItems: 'state/operator-notify-queue' },
           },
         },
-      },
+      }),
       repoRootOverride: repoRoot,
     }),
     /spec\.bindings\.scripts\.watchdog must stay within repo root: path escapes root boundary/
   );
+
+  assert.throws(
+    () => validateDeploymentProfileArtifact(createArtifact({
+      spec: {
+        package: { pluginVersion: '0.1.0-mainline' },
+        bindings: {
+          entrypoint: 'scripts/watchdog_auto_notify_orchestrator.mjs',
+          scripts: { watchdog: 'scripts/long_task_watchdog.mjs' },
+          artifact_roots: { queueItems: '../escape' },
+        },
+      },
+    })),
+    /spec\.bindings\.artifact_roots\.queueItems must stay within repo root: path escapes root boundary/
+  );
 });
 
 test('deployment binding contract allows normalized in-root paths that contain dot segments', () => {
   const binding = createDeploymentBindingContract({
-    artifact: {
+    artifact: createArtifact({
-      kind: 'DeploymentProfileArtifact',
-      apiVersion: 'reporting-governance/v1alpha1',
       spec: {
         package: { pluginVersion: '0.1.0-mainline' },
         bindings: {
@@ -194,7 +224,7 @@ test('deployment binding contract allows normalized in-root paths that contain d
           },
         },
       },
-    },
+    }),
     repoRootOverride: repoRoot,
   });
 
@@ -202,3 +232,30 @@ test('deployment binding contract allows normalized in-root paths that contain d
   assert.equal(binding.scripts.watchdog, path.resolve(repoRoot, 'scripts/long_task_watchdog.mjs'));
   assert.equal(binding.artifactRoots.queueItems, path.resolve(repoRoot, 'state/operator-notify-queue'));
 });
+
+test('deployment profile artifact validation rejects artifact_roots symlink escape after realpath resolution', async (t) => {
+  const sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'reporting-governance-profile-artifact-'));
+  t.after(() => fs.rmSync(sandbox, { recursive: true, force: true }));
+
+  const fakeRepoRoot = path.join(sandbox, 'repo');
+  const outsideRoot = path.join(sandbox, 'outside');
+  fs.mkdirSync(fakeRepoRoot, { recursive: true });
+  fs.mkdirSync(outsideRoot, { recursive: true });
+  fs.symlinkSync(outsideRoot, path.join(fakeRepoRoot, 'state-link'), 'dir');
+  fs.writeFileSync(path.join(fakeRepoRoot, 'entry.mjs'), 'export default true;\n');
+  fs.writeFileSync(path.join(fakeRepoRoot, 'watchdog.mjs'), 'export default true;\n');
+
+  assert.throws(
+    () => validateDeploymentProfileArtifact(createArtifact({
+      spec: {
+        package: { pluginVersion: '0.1.0-mainline' },
+        bindings: {
+          entrypoint: 'entry.mjs',
+          scripts: { watchdog: 'watchdog.mjs' },
+          artifact_roots: { queueItems: 'state-link/queue' },
+        },
+      },
+    }), { repoRootOverride: fakeRepoRoot }),
+    /spec\.bindings\.artifact_roots\.queueItems must stay within repo root: symlink resolution escapes realpath boundary/
+  );
+});