diff --git a/plugins/reporting-governance/src/storage/profile-artifact.mjs b/plugins/reporting-governance/src/storage/profile-artifact.mjs index 668064f..b49a696 100644 --- a/plugins/reporting-governance/src/storage/profile-artifact.mjs +++ b/plugins/reporting-governance/src/storage/profile-artifact.mjs @@ -24,7 +24,26 @@ function assertObjectRecord(value, label) { return value; } -export function validateDeploymentProfileArtifact(artifact) { +function assertRelativePathWithinRoot(relativePath, label, { root }) { + const normalizedPath = assertNonEmptyString(relativePath, label); + if (path.isAbsolute(normalizedPath)) { + throw new Error(`${label} must stay within repo root: absolute paths are not allowed`); + } + + const resolvedPath = path.resolve(root, normalizedPath); + const relativeToRoot = path.relative(root, resolvedPath); + if ( + relativeToRoot === '..' + || relativeToRoot.startsWith(`..${path.sep}`) + || path.isAbsolute(relativeToRoot) + ) { + throw new Error(`${label} must stay within repo root: path escapes root boundary`); + } + + return normalizedPath; +} + +export function validateDeploymentProfileArtifact(artifact, { repoRootOverride } = {}) { if (!artifact || typeof artifact !== 'object' || Array.isArray(artifact)) { throw new Error('deployment profile artifact must be an object'); } @@ -40,16 +59,18 @@ export function validateDeploymentProfileArtifact(artifact) { throw new Error('deployment profile artifact bindings are required'); } - assertNonEmptyString(bindings.entrypoint, 'deployment profile artifact spec.bindings.entrypoint'); + const root = path.resolve(repoRootOverride ?? repoRoot); const scripts = assertObjectRecord(bindings.scripts, 'deployment profile artifact spec.bindings.scripts'); const artifactRoots = assertObjectRecord(bindings.artifact_roots, 'deployment profile artifact spec.bindings.artifact_roots'); + + assertRelativePathWithinRoot(bindings.entrypoint, 'deployment profile artifact spec.bindings.entrypoint', { root }); assertNonEmptyString(artifact?.spec?.package?.pluginVersion, 'deployment profile artifact spec.package.pluginVersion'); for (const [key, relativePath] of Object.entries(scripts)) { - assertNonEmptyString(relativePath, `deployment profile artifact spec.bindings.scripts.${key}`); + assertRelativePathWithinRoot(relativePath, `deployment profile artifact spec.bindings.scripts.${key}`, { root }); } for (const [key, relativePath] of Object.entries(artifactRoots)) { - assertNonEmptyString(relativePath, `deployment profile artifact spec.bindings.artifact_roots.${key}`); + assertRelativePathWithinRoot(relativePath, `deployment profile artifact spec.bindings.artifact_roots.${key}`, { root }); } return artifact; @@ -72,7 +93,7 @@ export function loadDeploymentProfileArtifact({ artifactPath, profileId } = {}) } export function createDeploymentBindingContract({ artifact, repoRootOverride } = {}) { - const validatedArtifact = validateDeploymentProfileArtifact(artifact); + const validatedArtifact = validateDeploymentProfileArtifact(artifact, { repoRootOverride }); const root = path.resolve(repoRootOverride ?? repoRoot); const scripts = Object.fromEntries( Object.entries(validatedArtifact.spec.bindings.scripts).map(([key, relativePath]) => [key, path.resolve(root, relativePath)]) diff --git a/plugins/reporting-governance/test/profile-artifact.test.mjs b/plugins/reporting-governance/test/profile-artifact.test.mjs index dd9acfc..e65f0fc 100644 --- a/plugins/reporting-governance/test/profile-artifact.test.mjs +++ b/plugins/reporting-governance/test/profile-artifact.test.mjs @@ -104,3 +104,101 @@ test('deployment profile artifact validation fails closed on boundary drift', () /spec\.bindings\.scripts must be an object record/ ); }); + +test('deployment profile artifact validation rejects absolute binding paths', () => { + assert.throws( + () => validateDeploymentProfileArtifact({ + kind: 'DeploymentProfileArtifact', + apiVersion: 'reporting-governance/v1alpha1', + spec: { + package: { pluginVersion: '0.1.0-mainline' }, + bindings: { + entrypoint: '/abs/path', + scripts: { watchdog: 'scripts/long_task_watchdog.mjs' }, + artifact_roots: { queueItems: 'state/operator-notify-queue' }, + }, + }, + }), + /spec\.bindings\.entrypoint must stay within repo root: absolute paths are not allowed/ + ); + + assert.throws( + () => validateDeploymentProfileArtifact({ + kind: 'DeploymentProfileArtifact', + apiVersion: 'reporting-governance/v1alpha1', + spec: { + package: { pluginVersion: '0.1.0-mainline' }, + bindings: { + entrypoint: 'scripts/watchdog_auto_notify_orchestrator.mjs', + scripts: { watchdog: '/abs/path' }, + artifact_roots: { queueItems: 'state/operator-notify-queue' }, + }, + }, + }), + /spec\.bindings\.scripts\.watchdog must stay within repo root: absolute paths are not allowed/ + ); +}); + +test('deployment profile artifact validation rejects escape paths after resolution', () => { + assert.throws( + () => validateDeploymentProfileArtifact({ + kind: 'DeploymentProfileArtifact', + apiVersion: 'reporting-governance/v1alpha1', + spec: { + package: { pluginVersion: '0.1.0-mainline' }, + bindings: { + entrypoint: '../../escape', + scripts: { watchdog: 'scripts/long_task_watchdog.mjs' }, + artifact_roots: { queueItems: 'state/operator-notify-queue' }, + }, + }, + }), + /spec\.bindings\.entrypoint must stay within repo root: path escapes root boundary/ + ); + + assert.throws( + () => createDeploymentBindingContract({ + artifact: { + kind: 'DeploymentProfileArtifact', + apiVersion: 'reporting-governance/v1alpha1', + spec: { + package: { pluginVersion: '0.1.0-mainline' }, + bindings: { + entrypoint: 'scripts/watchdog_auto_notify_orchestrator.mjs', + scripts: { watchdog: '../../escape' }, + artifact_roots: { queueItems: 'state/operator-notify-queue' }, + }, + }, + }, + repoRootOverride: repoRoot, + }), + /spec\.bindings\.scripts\.watchdog 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: { + kind: 'DeploymentProfileArtifact', + apiVersion: 'reporting-governance/v1alpha1', + spec: { + package: { pluginVersion: '0.1.0-mainline' }, + bindings: { + runtime: 'openclaw', + entrypoint: 'scripts/../scripts/watchdog_auto_notify_orchestrator.mjs', + scripts: { + watchdog: 'scripts/./long_task_watchdog.mjs', + }, + artifact_roots: { + queueItems: 'state/../state/operator-notify-queue', + }, + }, + }, + }, + repoRootOverride: repoRoot, + }); + + assert.equal(binding.entrypoint, path.resolve(repoRoot, 'scripts/watchdog_auto_notify_orchestrator.mjs')); + 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')); +});