fix: skip invalid dependency names and versions

src/app/add-dependencies.ts

@@ -18,7 +18,7 @@ import {
   compareEditorVersion,
 } from "../domain/editor-version";
 import { partialApply } from "../domain/fp-utils";
-import type { DebugLog } from "../domain/logging";
+import type { DebugLog, WarnLog } from "../domain/logging";
 import { tryGetTargetEditorVersionFor } from "../domain/package-manifest";
 import {
   type PackageSpec,
@@ -151,6 +151,7 @@ function pickMostFixable(
  * @param fetchPackument IO function for fetching remote packuments.
  * @param checkUrlExists  IO function for checking whether a URL exists.
  * @param debugLog IO function for printing debug logs.
+ * @param warnLog IO function for printing user-facing warnings.
  * @param projectDirectory The projects root directory.
  * @param editorVersion The projects editor version. Will be used to check
  * compatibility. If set to null then compatibility will not be checked.
@@ -166,6 +167,7 @@ export async function addDependenciesUsing(
   fetchPackument: GetRegistryPackument,
   checkUrlExists: CheckUrlExists,
   debugLog: DebugLog,
+  warnLog: WarnLog,
   projectDirectory: string,
   editorVersion: ReleaseVersion | null,
   sources: ReadonlyArray<Registry>,
@@ -192,7 +194,8 @@ export async function addDependenciesUsing(
   const resolveDependencies = partialApply(
     resolveDependenciesUsing,
     checkUrlExists,
-    fetchPackument
+    fetchPackument,
+    warnLog
   );
 
   async function resolveScopesFor(

src/app/resolve-dependencies.ts

@@ -9,13 +9,16 @@ import {
 } from "../domain/dependency-graph";
 import { DomainName } from "../domain/domain-name";
 import { partialApply } from "../domain/fp-utils";
+import { type WarnLog } from "../domain/logging";
 import {
   ResolvePackumentVersionError,
   tryResolvePackumentVersion,
 } from "../domain/packument";
+import { recordEntries } from "../domain/record-utils";
 import { Registry } from "../domain/registry";
 import { RegistryUrl } from "../domain/registry-url";
 import { SemanticVersion } from "../domain/semantic-version";
+import { isZod } from "../domain/zod-utils";
 import type { GetRegistryPackument } from "../io/registry";
 import { type CheckUrlExists } from "../io/www";
 import { checkIsBuiltInPackageUsing } from "./built-in-package-check";
@@ -24,6 +27,7 @@ import { checkIsBuiltInPackageUsing } from "./built-in-package-check";
  * Resolves all dependencies for a package.
  * @param checkUrlExists IO function for checking whether a package exists.
  * @param getRegistryPackument IO function for fetching a registry packument.
+ * @param warnLog IO function for logging user-facing warnings.
  * @param sources Sources from which dependencies can be resolved.
  * @param packageName The name of the package.
  * @param version The version for which to search dependencies.
@@ -32,6 +36,7 @@ import { checkIsBuiltInPackageUsing } from "./built-in-package-check";
 export function resolveDependenciesUsing(
   checkUrlExists: CheckUrlExists,
   getRegistryPackument: GetRegistryPackument,
+  warnLog: WarnLog,
   sources: ReadonlyArray<Registry>,
   packageName: DomainName,
   version: SemanticVersion,
@@ -78,12 +83,18 @@ export function resolveDependenciesUsing(
       }
 
       const dependencies = packumentVersionResult.value.dependencies ?? {};
+      const validDependencies = await makeValidDependencies(
+        dependencies,
+        currentPackageName,
+        currentVersion,
+        warnLog
+      );
       graph = markRemoteResolved(
         graph,
         currentPackageName,
         currentVersion,
         source.url,
-        dependencies
+        validDependencies
       );
       if (!deep) return graph;
 
@@ -96,3 +107,38 @@ export function resolveDependenciesUsing(
 
   return resolveRecursively(makeGraphFromSeed(packageName, version));
 }
+
+/**
+ * Filters out dependencies whose names or versions are invalid.
+ * Registry packuments are untrusted input, so dependency names and versions
+ * must be validated before they are used to build package specs or dependency
+ * graphs. Invalid dependencies are skipped with a warning, which matches how
+ * the Unity Package Manager handles them.
+ * @param dependencies The dependencies declared by a package version.
+ * @param packageName The name of the package that declares the dependencies.
+ * @param version The version of the package that declares the dependencies.
+ * @param warnLog IO function for logging user-facing warnings.
+ */
+async function makeValidDependencies(
+  dependencies: Readonly<Record<DomainName, SemanticVersion>>,
+  packageName: DomainName,
+  version: SemanticVersion,
+  warnLog: WarnLog
+): Promise<Record<DomainName, SemanticVersion>> {
+  const validDependencies: Record<DomainName, SemanticVersion> = {};
+  for (const [dependencyName, dependencyVersion] of recordEntries(
+    dependencies
+  )) {
+    if (
+      !isZod(dependencyName, DomainName) ||
+      !isZod(dependencyVersion, SemanticVersion)
+    ) {
+      await warnLog(
+        `Skipping invalid dependency "${dependencyName}": "${dependencyVersion}" declared by "${packageName}@${version}". Dependency names and versions must follow the Unity package naming and semantic versioning rules. Report this to the package author.`
+      );
+      continue;
+    }
+    validDependencies[dependencyName] = dependencyVersion;
+  }
+  return validDependencies;
+}

src/cli/cmd-add.ts

@@ -4,7 +4,7 @@ import { addDependenciesUsing } from "../app/add-dependencies";
 import { determineEditorVersionUsing } from "../app/determine-editor-version";
 import { loadRegistryAuthUsing } from "../app/get-registry-auth";
 import { partialApply } from "../domain/fp-utils";
-import { DebugLog } from "../domain/logging";
+import { DebugLog, WarnLog } from "../domain/logging";
 import { makePackageSpec } from "../domain/package-spec";
 import { recordEntries } from "../domain/record-utils";
 import { unityRegistry } from "../domain/registry";
@@ -67,13 +67,16 @@ export function makeAddCmd(
     debugLog
   );
 
+  const warnLog: WarnLog = async (message) => log.warn("", message);
+
   const addDependencies = partialApply(
     addDependenciesUsing,
     readTextFile,
     writeTextFile,
     fetchPackument,
     checkUrlExists,
-    debugLog
+    debugLog,
+    warnLog
   );
 
   return new Command("add")

src/cli/cmd-deps.ts

@@ -8,7 +8,7 @@ import { queryAllRegistriesLazy } from "../app/query-registries";
 import { resolveDependenciesUsing } from "../app/resolve-dependencies";
 import { PackumentNotFoundError } from "../domain/common-errors";
 import { partialApply } from "../domain/fp-utils";
-import { DebugLog } from "../domain/logging";
+import { DebugLog, WarnLog } from "../domain/logging";
 import { makePackageSpec, splitPackageSpec } from "../domain/package-spec";
 import { PackageUrl } from "../domain/package-url";
 import { unityRegistry } from "../domain/registry";
@@ -62,10 +62,13 @@ export function makeDepsCmd(
     fetchPackument
   );
 
+  const warnLog: WarnLog = async (message) => log.warn("", message);
+
   const resolveDependencies = partialApply(
     resolveDependenciesUsing,
     checkUrlExists,
-    fetchPackument
+    fetchPackument,
+    warnLog
   );
 
   return new Command("deps")

src/cli/error-logging.ts

@@ -41,6 +41,8 @@ function errorMessageFor(error: ResolvePackumentVersionError): string {
   else return "version not found";
 }
 
+const unrecognizedErrorMessage = "A fatal error occurred.";
+
 function stringifyUnresolvedDependency(
   dependency: UnresolvedDependency
 ): string {
@@ -53,7 +55,7 @@ function stringifyUnresolvedDependency(
   );
 }
 
-function makeErrorMessageFor(error: unknown): string {
+function makeErrorMessageFor(error: unknown): string | null {
   if (error instanceof PackumentNotFoundError)
     return `Package "${error.packageName}" could not be found.`;
   if (error instanceof EditorVersionNotSupportedError)
@@ -99,7 +101,7 @@ function makeErrorMessageFor(error: unknown): string {
     return "Could not determine path of system user directory.";
   if (error instanceof NoStableError)
     return "Seems like the package you requested has no stable versions.";
-  return "A fatal error occurred.";
+  return null;
 }
 
 function tryMakeFixSuggestionFor(error: unknown): string | null {
@@ -138,21 +140,38 @@ function tryMakeFixSuggestionFor(error: unknown): string | null {
   return null;
 }
 
+/**
+ * Formats an error and its causes for verbose output.
+ * @param error The error to format.
+ * @param depth The current cause depth. Used to avoid endless recursion.
+ */
+function formatError(error: unknown, depth = 0): string {
+  if (!(error instanceof Error) || depth > 3) return String(error);
+  const message = error.stack ?? error.message;
+  if (error.cause === undefined) return message;
+  return `${message}${EOL}Caused by: ${formatError(error.cause, depth + 1)}`;
+}
+
 /**
  * Prints information about an error to a logger.
  * @param log The logger to print to.
  * @param error The error.
  */
 export function logError(log: Logger, error: unknown) {
   const message = makeErrorMessageFor(error);
-  log.error("", message);
+  log.error("", message ?? unrecognizedErrorMessage);
 
   const fixSuggestion = tryMakeFixSuggestionFor(error);
   if (fixSuggestion !== null) log.notice("", fixSuggestion);
 
   const isLoggingVerbose = log.level === "verbose" || log.level === "silly";
-  if (!isLoggingVerbose)
+  if (isLoggingVerbose) {
+    // Unrecognized errors only show a generic message to users. In verbose
+    // mode the underlying cause is printed so that the error is diagnosable.
+    if (message === null) log.error("", formatError(error));
+  } else {
     log.notice("", "Run with --verbose to get more information.");
+  }
 }
 
 /**

src/domain/logging.ts

@@ -13,3 +13,15 @@ export type DebugLog = (message: string, context?: object) => Promise<void>;
  * A {@link DebugLog} which does nothing.
  */
 export const noopLogger: DebugLog = async () => {};
+
+/**
+ * Function that logs a user-facing warning. This interface is output-agnostic
+ * meaning that the logs could go to a console, file or other.
+ * @param message The message to be logged.
+ */
+export type WarnLog = (message: string) => Promise<void>;
+
+/**
+ * A {@link WarnLog} which does nothing.
+ */
+export const noopWarnLog: WarnLog = async () => {};

test/common/data-packument.ts

@@ -29,6 +29,31 @@ class UnityPackumentVersionBuilder {
   addDependency(name: string, version: string): UnityPackumentVersionBuilder {
     assert(isZod(name, DomainName), `${name} is domain name`);
     assert(isZod(version, SemanticVersion), `${version} is semantic version`);
+    return this.putDependency(name, version);
+  }
+
+  /**
+   * Add a dependency without validating its name or version. Useful for
+   * testing how malformed packuments are handled.
+   * @param name The name of the dependency.
+   * @param version The version of the dependency.
+   */
+  addDependencyUnchecked(
+    name: string,
+    version: string
+  ): UnityPackumentVersionBuilder {
+    return this.putDependency(name, version);
+  }
+
+  /**
+   * Sets a dependency on this version.
+   * @param name The name of the dependency.
+   * @param version The version of the dependency.
+   */
+  private putDependency(
+    name: string,
+    version: string
+  ): UnityPackumentVersionBuilder {
     this.version = {
       ...this.version,
       dependencies: {