From 9475bccaed865ae8b654b52a9554e4283b24695a Mon Sep 17 00:00:00 2001
From: Miguel Lezama <lezama@gmail.com>
Date: Mon, 3 Aug 2026 11:29:49 -0300
Subject: [PATCH] Security: unify package artifact-id normalization and reject
 path traversal

Package artifact-id / source-path normalization was duplicated across four
sites with three incompatible rule-sets, and the adoption orchestrator's copy
validated nothing at all -- an unauthenticated path-traversal seam (CWE-22 /
CWE-20) into the artifact_key it builds.

Divergent copies unified behind one shared normalizer
(WP_Agent_Package_Artifact_Identity):

- src/Packages/class-wp-agent-package-adoption-orchestrator.php artifact_id():
  only trimmed and normalized backslashes -- NO leading-slash check, NO ".."
  check. This built artifact_key, so it was the loose end (the no-op copy).
- src/Packages/class-wp-agent-package-update-planner.php normalize_artifact_id():
  rejected leading "/" and str_contains("..") -- too strict, also rejected the
  legitimate name "a..b".
- src/Packages/class-wp-agent-package-installed-artifact.php prepare_id():
  rejected str_contains("..") -- same over-broad rule.
- src/Packages/class-wp-agent-package-artifact.php prepare_source():
  rejected ".." only as a whole path segment (correct) plus a drive-letter
  guard, so "a..b" passed here but was rejected by the planner.

The shared normalizer applies the strictest CORRECT rule uniformly: reject a
leading "/", reject any ".." as a whole traversal segment (so "a..b" stays a
valid name), normalize backslashes before the segment check, and keep the
drive-letter guard on sources. All four call sites now route through it.

Adds tests/package-artifact-id-normalization-smoke.php (registered in the
composer "smoke" array), which fails without this fix: it asserts traversal /
absolute ids (../x, /abs, a/../b, plus backslash variants) are rejected at
every seam, the a..b segment edge is accepted consistently, and legitimate ids
(foo, foo/bar, foo-bar_baz, memory/agent/SOUL.md) pass unchanged.

composer smoke green; phpstan analyse reports no errors.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 agents-api.php                                |   1 +
 composer.json                                 |   1 +
 ...wp-agent-package-adoption-orchestrator.php |   2 +-
 ...ass-wp-agent-package-artifact-identity.php | 118 +++++++++++
 .../class-wp-agent-package-artifact.php       |  21 +-
 ...ss-wp-agent-package-installed-artifact.php |   7 +-
 .../class-wp-agent-package-update-planner.php |   7 +-
 ...ackage-artifact-id-normalization-smoke.php | 198 ++++++++++++++++++
 tests/run-result-envelope-smoke.php           |   1 +
 9 files changed, 323 insertions(+), 33 deletions(-)
 create mode 100644 src/Packages/class-wp-agent-package-artifact-identity.php
 create mode 100644 tests/package-artifact-id-normalization-smoke.php

diff --git a/agents-api.php b/agents-api.php
index 4210bd90..629e3777 100644
--- a/agents-api.php
+++ b/agents-api.php
@@ -102,6 +102,7 @@
 require_once AGENTS_API_PATH . 'src/Registry/class-wp-agent-installed-agent-state-store.php';
 require_once AGENTS_API_PATH . 'src/Registry/class-wp-agent-installed-agent-projector.php';
 require_once AGENTS_API_PATH . 'src/Registry/class-wp-agent-registered-agent-materialization-adapter.php';
+require_once AGENTS_API_PATH . 'src/Packages/class-wp-agent-package-artifact-identity.php';
 require_once AGENTS_API_PATH . 'src/Packages/class-wp-agent-package-artifact.php';
 require_once AGENTS_API_PATH . 'src/Packages/class-wp-agent-package-artifact-type.php';
 require_once AGENTS_API_PATH . 'src/Packages/class-wp-agent-package-artifacts-registry.php';
diff --git a/composer.json b/composer.json
index 8b864bfa..14cf96e0 100644
--- a/composer.json
+++ b/composer.json
@@ -36,6 +36,7 @@
       "php tests/runtime-agent-bundle-importer-smoke.php",
       "php tests/package-lifecycle-smoke.php",
       "php tests/package-duplicate-artifact-identity-smoke.php",
+      "php tests/package-artifact-id-normalization-smoke.php",
       "php tests/package-capability-contract-smoke.php",
       "php tests/package-adoption-orchestration-smoke.php",
       "php tests/execution-principal-smoke.php",
diff --git a/src/Packages/class-wp-agent-package-adoption-orchestrator.php b/src/Packages/class-wp-agent-package-adoption-orchestrator.php
index 9fdce3dd..58be27b9 100644
--- a/src/Packages/class-wp-agent-package-adoption-orchestrator.php
+++ b/src/Packages/class-wp-agent-package-adoption-orchestrator.php
@@ -202,7 +202,7 @@ private static function snapshot_from_target( WP_Agent_Package $package, array $
 
 		/** @param array<string,mixed> $artifact */
 		private static function artifact_id( array $artifact ): string {
-			return trim( str_replace( '\\', '/', self::string_value( $artifact['artifact_id'] ?? ( $artifact['slug'] ?? '' ) ) ) );
+			return WP_Agent_Package_Artifact_Identity::normalize_id( $artifact['artifact_id'] ?? ( $artifact['slug'] ?? '' ) );
 		}
 
 		private static function artifact_key( string $type, string $id ): string {
diff --git a/src/Packages/class-wp-agent-package-artifact-identity.php b/src/Packages/class-wp-agent-package-artifact-identity.php
new file mode 100644
index 00000000..8c1b5ae0
--- /dev/null
+++ b/src/Packages/class-wp-agent-package-artifact-identity.php
@@ -0,0 +1,118 @@
+<?php
+/**
+ * WP_Agent_Package_Artifact_Identity normalizer.
+ *
+ * @package AgentsAPI
+ */
+
+defined( 'ABSPATH' ) || exit;
+
+if ( ! class_exists( 'WP_Agent_Package_Artifact_Identity' ) ) {
+	/**
+	 * Shared normalizer for package artifact identifiers and source paths.
+	 *
+	 * Every place that derives a package-local key or path routes through here so
+	 * the traversal/absolute-path rules stay uniform across the update planner,
+	 * installed-artifact snapshot, artifact declaration, and adoption orchestrator.
+	 */
+	final class WP_Agent_Package_Artifact_Identity {
+
+		/**
+		 * Normalizes a package-local artifact identifier.
+		 *
+		 * Rejects empty, absolute (leading slash), and parent-directory traversal
+		 * segments. Backslashes are normalized to forward slashes so Windows-style
+		 * separators cannot smuggle a traversal past the segment check.
+		 *
+		 * @param mixed $value Raw artifact identifier.
+		 * @return string Package-local identifier.
+		 * @throws InvalidArgumentException When the identifier is empty, absolute, or traverses.
+		 */
+		public static function normalize_id( mixed $value ): string {
+			$id = self::normalize_separators( $value );
+			if ( '' === $id || str_starts_with( $id, '/' ) || self::has_traversal_segment( $id ) ) {
+				throw new InvalidArgumentException( 'Agent package artifact identifier must be a non-empty package-local path without parent directory traversal.' );
+			}
+
+			return $id;
+		}
+
+		/**
+		 * Normalizes a package-relative source path.
+		 *
+		 * An empty source is allowed (the artifact declares no payload location).
+		 * A non-empty source must be relative (no leading slash, no drive letter)
+		 * and must not contain a parent-directory segment. Empty segments are
+		 * collapsed so `a//b` normalizes to `a/b`.
+		 *
+		 * @param mixed $value Raw source path.
+		 * @return string Package-relative source path, or an empty string.
+		 * @throws InvalidArgumentException When the source is absolute, drive-anchored, or traverses.
+		 */
+		public static function normalize_source( mixed $value ): string {
+			$source = self::normalize_separators( $value );
+			if ( '' === $source ) {
+				return '';
+			}
+
+			if ( str_starts_with( $source, '/' ) || preg_match( '/^[A-Za-z]:\//', $source ) ) {
+				throw new InvalidArgumentException( 'Agent package artifact source must be relative to the package.' );
+			}
+
+			$parts = array_values(
+				array_filter(
+					explode( '/', $source ),
+					static function ( string $part ): bool {
+						return '' !== $part;
+					}
+				)
+			);
+			if ( in_array( '..', $parts, true ) ) {
+				throw new InvalidArgumentException( 'Agent package artifact source cannot contain parent directory segments.' );
+			}
+
+			return implode( '/', $parts );
+		}
+
+		/**
+		 * Determines whether any path segment is a parent-directory traversal.
+		 *
+		 * `..` is only dangerous as a whole segment; `a..b` is a legitimate name.
+		 *
+		 * @param string $path Slash-separated path.
+		 * @return bool
+		 */
+		private static function has_traversal_segment( string $path ): bool {
+			return in_array( '..', explode( '/', $path ), true );
+		}
+
+		/**
+		 * Trims and converts backslashes to forward slashes.
+		 *
+		 * @param mixed $value Raw value.
+		 * @return string
+		 */
+		private static function normalize_separators( mixed $value ): string {
+			return trim( str_replace( '\\', '/', self::string_value( $value ) ) );
+		}
+
+		/**
+		 * Convert scalar/Stringable input to a string.
+		 *
+		 * @param mixed $value Raw value.
+		 * @return string String value, or empty string for non-stringable input.
+		 */
+		private static function string_value( mixed $value ): string {
+			if ( null === $value ) {
+				return '';
+			}
+
+			return is_scalar( $value ) || $value instanceof Stringable ? (string) $value : '';
+		}
+
+		/**
+		 * Prevents construction.
+		 */
+		private function __construct() {}
+	}
+}
diff --git a/src/Packages/class-wp-agent-package-artifact.php b/src/Packages/class-wp-agent-package-artifact.php
index 5143d6c2..2b1d445c 100644
--- a/src/Packages/class-wp-agent-package-artifact.php
+++ b/src/Packages/class-wp-agent-package-artifact.php
@@ -239,26 +239,7 @@ private function prepare_slug( $slug ): string {
 		 * @return string
 		 */
 		private function prepare_source( $source ): string {
-			$source = trim( str_replace( '\\', '/', self::string_value( $source ) ) );
-			if ( '' === $source ) {
-				return '';
-			}
-
-			if ( str_starts_with( $source, '/' ) || preg_match( '/^[A-Za-z]:\//', $source ) ) {
-				throw new InvalidArgumentException( 'Agent package artifact source must be relative to the package.' );
-			}
-
-			$parts = array_filter(
-				explode( '/', $source ),
-				static function ( string $part ): bool {
-					return '' !== $part;
-				}
-			);
-			if ( in_array( '..', $parts, true ) ) {
-				throw new InvalidArgumentException( 'Agent package artifact source cannot contain parent directory segments.' );
-			}
-
-			return implode( '/', $parts );
+			return WP_Agent_Package_Artifact_Identity::normalize_source( $source );
 		}
 
 		/**
diff --git a/src/Packages/class-wp-agent-package-installed-artifact.php b/src/Packages/class-wp-agent-package-installed-artifact.php
index 1694e3e8..c4d49648 100644
--- a/src/Packages/class-wp-agent-package-installed-artifact.php
+++ b/src/Packages/class-wp-agent-package-installed-artifact.php
@@ -182,12 +182,7 @@ private function prepare_slug( mixed $value, string $field ): string {
 		}
 
 		private function prepare_id( mixed $value ): string {
-			$value = trim( str_replace( '\\', '/', $this->string_value( $value ) ) );
-			if ( '' === $value || str_starts_with( $value, '/' ) || str_contains( $value, '..' ) ) {
-				throw new InvalidArgumentException( 'Agent package installed artifact artifact_id must be a non-empty package-local identifier.' );
-			}
-
-			return $value;
+			return WP_Agent_Package_Artifact_Identity::normalize_id( $value );
 		}
 
 		private function prepare_string( mixed $value, string $field ): string {
diff --git a/src/Packages/class-wp-agent-package-update-planner.php b/src/Packages/class-wp-agent-package-update-planner.php
index 4314ed46..613645fe 100644
--- a/src/Packages/class-wp-agent-package-update-planner.php
+++ b/src/Packages/class-wp-agent-package-update-planner.php
@@ -174,12 +174,7 @@ private static function artifact_key( string $type, string $slug ): string {
 		}
 
 		private static function normalize_artifact_id( mixed $artifact_id ): string {
-			$artifact_id = trim( str_replace( '\\', '/', self::string_value( $artifact_id ) ) );
-			if ( '' === $artifact_id || str_starts_with( $artifact_id, '/' ) || str_contains( $artifact_id, '..' ) ) {
-				throw new InvalidArgumentException( 'Agent package artifact rows require a package-local artifact_id.' );
-			}
-
-			return $artifact_id;
+			return WP_Agent_Package_Artifact_Identity::normalize_id( $artifact_id );
 		}
 
 		/** @param array<string,mixed>|null $artifact */
diff --git a/tests/package-artifact-id-normalization-smoke.php b/tests/package-artifact-id-normalization-smoke.php
new file mode 100644
index 00000000..bb1688bb
--- /dev/null
+++ b/tests/package-artifact-id-normalization-smoke.php
@@ -0,0 +1,198 @@
+<?php
+/**
+ * Pure-PHP smoke test asserting uniform package artifact-id normalization.
+ *
+ * Artifact-id / source-path normalization used to be duplicated across four
+ * sites with three incompatible rule-sets, and the adoption orchestrator's
+ * copy validated nothing at all -- a path-traversal seam (CWE-22/-20). This
+ * test pins every seam to one shared rule: reject absolute (leading-slash) and
+ * parent-directory-traversal identifiers, treat `..` as dangerous only as a
+ * whole path segment (so `a..b` is a legitimate name), and never regress
+ * ordinary package-local ids.
+ *
+ * Run with: php tests/package-artifact-id-normalization-smoke.php
+ *
+ * @package AgentsAPI\Tests
+ */
+
+if ( ! defined( 'ABSPATH' ) ) {
+	define( 'ABSPATH', __DIR__ . '/' );
+}
+
+$failures = array();
+$passes   = 0;
+
+echo "agents-api-package-artifact-id-normalization-smoke\n";
+
+require_once __DIR__ . '/agents-api-smoke-helpers.php';
+agents_api_smoke_require_module();
+
+/**
+ * Runs a normalizer and reports whether it accepted or rejected the input.
+ *
+ * @param callable $fn    Normalizer under test.
+ * @param string   $input Raw identifier or source path.
+ * @return string 'rejected' or 'accepted:<normalized value>'.
+ */
+function agents_api_probe_normalizer( callable $fn, string $input ): string {
+	try {
+		return 'accepted:' . $fn( $input );
+	} catch ( InvalidArgumentException $e ) {
+		return 'rejected';
+	}
+}
+
+// Identifier seams. The orchestrator copy is private, so reach it by reflection
+// to prove the previously-unvalidated artifact_key path is now closed.
+$orchestrator_id = static function ( string $id ): string {
+	$method = new ReflectionMethod( 'WP_Agent_Package_Adoption_Orchestrator', 'artifact_id' );
+	$method->setAccessible( true );
+	return (string) $method->invoke( null, array( 'artifact_id' => $id ) );
+};
+
+$planner_id = static function ( string $id ): string {
+	$method = new ReflectionMethod( 'WP_Agent_Package_Update_Planner', 'normalize_artifact_id' );
+	$method->setAccessible( true );
+	return (string) $method->invoke( null, $id );
+};
+
+$installed_id = static function ( string $id ): string {
+	$artifact = new WP_Agent_Package_Installed_Artifact(
+		array(
+			'package_slug'    => 'demo-package',
+			'package_version' => '1.0.0',
+			'artifact_type'   => 'example/prompt',
+			'artifact_id'     => $id,
+			'source'          => 'prompts/ok.md',
+			'installed_at'    => '2026-05-25T00:00:00Z',
+			'updated_at'      => '2026-05-25T00:00:00Z',
+		)
+	);
+	return $artifact->get_artifact_id();
+};
+
+$shared_id = static function ( string $id ): string {
+	return WP_Agent_Package_Artifact_Identity::normalize_id( $id );
+};
+
+$id_normalizers = array(
+	'orchestrator' => $orchestrator_id,
+	'planner'      => $planner_id,
+	'installed'    => $installed_id,
+	'shared'       => $shared_id,
+);
+
+// Source seams share the traversal/absolute rule but allow an empty value and
+// guard against drive-letter anchors.
+$artifact_source = static function ( string $source ): string {
+	$artifact = new WP_Agent_Package_Artifact(
+		array(
+			'type'   => 'example/prompt',
+			'slug'   => 'demo',
+			'source' => $source,
+		)
+	);
+	return $artifact->get_source();
+};
+
+$shared_source = static function ( string $source ): string {
+	return WP_Agent_Package_Artifact_Identity::normalize_source( $source );
+};
+
+$source_normalizers = array(
+	'artifact-source' => $artifact_source,
+	'shared-source'   => $shared_source,
+);
+
+echo "\n[1] Traversal and absolute identifiers are rejected at every seam:\n";
+$traversal_inputs = array( '../x', '/abs', 'a/../b', '..\\x', 'a\\..\\b' );
+foreach ( $traversal_inputs as $input ) {
+	foreach ( $id_normalizers as $label => $fn ) {
+		agents_api_smoke_assert_equals(
+			'rejected',
+			agents_api_probe_normalizer( $fn, $input ),
+			sprintf( '%s rejects traversal/absolute id %s', $label, var_export( $input, true ) ),
+			$failures,
+			$passes
+		);
+	}
+	foreach ( $source_normalizers as $label => $fn ) {
+		agents_api_smoke_assert_equals(
+			'rejected',
+			agents_api_probe_normalizer( $fn, $input ),
+			sprintf( '%s rejects traversal/absolute source %s', $label, var_export( $input, true ) ),
+			$failures,
+			$passes
+		);
+	}
+}
+
+echo "\n[2] The `a..b` segment edge is accepted consistently (not treated as traversal):\n";
+foreach ( $id_normalizers as $label => $fn ) {
+	agents_api_smoke_assert_equals(
+		'accepted:a..b',
+		agents_api_probe_normalizer( $fn, 'a..b' ),
+		sprintf( '%s accepts non-traversal a..b unchanged', $label ),
+		$failures,
+		$passes
+	);
+}
+foreach ( $source_normalizers as $label => $fn ) {
+	agents_api_smoke_assert_equals(
+		'accepted:a..b',
+		agents_api_probe_normalizer( $fn, 'a..b' ),
+		sprintf( '%s accepts non-traversal a..b source unchanged', $label ),
+		$failures,
+		$passes
+	);
+}
+
+echo "\n[3] Legitimate package-local identifiers still pass unchanged:\n";
+$legit_inputs = array( 'foo', 'foo/bar', 'foo-bar_baz', 'memory/agent/SOUL.md' );
+foreach ( $legit_inputs as $input ) {
+	foreach ( $id_normalizers as $label => $fn ) {
+		agents_api_smoke_assert_equals(
+			'accepted:' . $input,
+			agents_api_probe_normalizer( $fn, $input ),
+			sprintf( '%s preserves legitimate id %s', $label, var_export( $input, true ) ),
+			$failures,
+			$passes
+		);
+	}
+	foreach ( $source_normalizers as $label => $fn ) {
+		agents_api_smoke_assert_equals(
+			'accepted:' . $input,
+			agents_api_probe_normalizer( $fn, $input ),
+			sprintf( '%s preserves legitimate source %s', $label, var_export( $input, true ) ),
+			$failures,
+			$passes
+		);
+	}
+}
+
+echo "\n[4] Backslash separators normalize before the segment check:\n";
+agents_api_smoke_assert_equals(
+	'accepted:foo/bar',
+	agents_api_probe_normalizer( $shared_id, 'foo\\bar' ),
+	'shared normalizer converts backslashes to forward slashes',
+	$failures,
+	$passes
+);
+
+echo "\n[5] Empty source is allowed; empty id is rejected:\n";
+agents_api_smoke_assert_equals(
+	'accepted:',
+	agents_api_probe_normalizer( $shared_source, '' ),
+	'empty source normalizes to an empty string',
+	$failures,
+	$passes
+);
+agents_api_smoke_assert_equals(
+	'rejected',
+	agents_api_probe_normalizer( $shared_id, '' ),
+	'empty id is rejected',
+	$failures,
+	$passes
+);
+
+agents_api_smoke_finish( 'Agents API package artifact-id normalization', $failures, $passes );
diff --git a/tests/run-result-envelope-smoke.php b/tests/run-result-envelope-smoke.php
index 7734d9d7..22bda0a3 100644
--- a/tests/run-result-envelope-smoke.php
+++ b/tests/run-result-envelope-smoke.php
@@ -19,6 +19,7 @@
 require_once __DIR__ . '/../src/Runtime/class-wp-agent-runtime-package-run-result.php';
 require_once __DIR__ . '/../src/Workflows/class-wp-agent-workflow-run-result.php';
 require_once __DIR__ . '/../src/Tasks/class-wp-agent-task-run-control.php';
+require_once __DIR__ . '/../src/Packages/class-wp-agent-package-artifact-identity.php';
 require_once __DIR__ . '/../src/Packages/class-wp-agent-package-artifact.php';
 require_once __DIR__ . '/../src/Packages/class-wp-agent-package-artifact-status.php';
 require_once __DIR__ . '/../src/Packages/class-wp-agent-package-installed-artifact.php';