add env validation (#522)

Author: formaceft-93

packages/cli/src/commands/qualityGate.ts

@@ -3,8 +3,8 @@ import { realpath } from "node:fs/promises";
 import { exit, cwd as processCwd } from "node:process";
 
 import { AllureReport, QualityGateState, readConfig, stringifyQualityGateResults } from "@allurereport/core";
-import type { TestResult } from "@allurereport/core-api";
-import { Command, Option } from "clipanion";
+import { type TestResult, validateEnvironmentName } from "@allurereport/core-api";
+import { Command, Option, UsageError } from "clipanion";
 import { glob } from "glob";
 import * as typanion from "typanion";
 import { red } from "yoctocolors";
@@ -66,9 +66,23 @@ export class QualityGateCommand extends Command {
   });
 
   async execute() {
+    let normalizedEnvironment = this.environment;
+
+    if (typeof this.environment === "string") {
+      const envValidationResult = validateEnvironmentName(this.environment);
+
+      if (!envValidationResult.valid) {
+        throw new UsageError(
+          `Invalid --environment value ${JSON.stringify(this.environment)}: ${envValidationResult.reason}`,
+        );
+      }
+
+      normalizedEnvironment = envValidationResult.normalized;
+    }
+
     const cwd = await realpath(this.cwd ?? processCwd());
     const resultsDir = (this.resultsDir ?? "./**/allure-results").replace(/[\\/]$/, "");
-    const { maxFailures, minTestsCount, successRate, fastFail, knownIssues: knownIssuesPath, environment } = this;
+    const { maxFailures, minTestsCount, successRate, fastFail, knownIssues: knownIssuesPath } = this;
     const config = await readConfig(cwd, this.config, {
       knownIssuesPath,
     });
@@ -139,7 +153,7 @@ export class QualityGateCommand extends Command {
       const notHiddenTrs = (trs as TestResult[]).filter((tr) => !tr.hidden);
       const { results, fastFailed } = await allureReport.validate({
         trs: notHiddenTrs,
-        environment,
+        environment: normalizedEnvironment,
         knownIssues,
         state,
       });
@@ -166,7 +180,7 @@ export class QualityGateCommand extends Command {
     const validationResults = await allureReport.validate({
       trs: allTrs,
       knownIssues,
-      environment,
+      environment: normalizedEnvironment,
     });
 
     if (validationResults.results.length === 0) {

packages/cli/src/commands/run.ts

@@ -12,7 +12,7 @@ import {
   readConfig,
   stringifyQualityGateResults,
 } from "@allurereport/core";
-import { type KnownTestFailure, createTestPlan } from "@allurereport/core-api";
+import { type KnownTestFailure, createTestPlan, validateEnvironmentName } from "@allurereport/core-api";
 import type { Watcher } from "@allurereport/directory-watcher";
 import {
   allureResultsDirectoriesWatcher,
@@ -24,7 +24,7 @@ import Awesome from "@allurereport/plugin-awesome";
 import { BufferResultFile, PathResultFile } from "@allurereport/reader-api";
 import { KnownError } from "@allurereport/service";
 import { serve } from "@allurereport/static-server";
-import { Command, Option } from "clipanion";
+import { Command, Option, UsageError } from "clipanion";
 import { red } from "yoctocolors";
 
 import { logTests, runProcess, terminationOf } from "../utils/index.js";
@@ -297,7 +297,21 @@ export class RunCommand extends Command {
     const args = this.commandToRun.filter((arg) => arg !== "--") as string[] | undefined;
 
     if (!args || !args.length) {
-      throw new Error("expecting command to be specified after --, e.g. allure run -- npm run test");
+      throw new UsageError("expecting command to be specified after --, e.g. allure run -- npm run test");
+    }
+
+    let normalizedEnvironment = this.environment;
+
+    if (this.environment !== undefined) {
+      const envValidationResult = validateEnvironmentName(this.environment);
+
+      if (!envValidationResult.valid) {
+        throw new UsageError(
+          `invalid --environment value ${JSON.stringify(this.environment)}: ${envValidationResult.reason}`,
+        );
+      }
+
+      normalizedEnvironment = envValidationResult.normalized;
     }
 
     const before = new Date().getTime();
@@ -322,6 +336,7 @@ export class RunCommand extends Command {
       port: this.port,
       historyLimit: this.historyLimit ? parseInt(this.historyLimit, 10) : undefined,
     });
+    const resolvedEnvironment = normalizedEnvironment ?? config.environment;
     const withQualityGate = !!config.qualityGate;
     const withRerun = !!this.rerun;
 
@@ -340,7 +355,7 @@ export class RunCommand extends Command {
     }
     const allureReport = new AllureReport({
       ...config,
-      environment: this.environment,
+      environment: resolvedEnvironment,
       dump: this.dump,
       realTime: false,
       plugins: [
@@ -378,7 +393,7 @@ export class RunCommand extends Command {
         cwd,
         command,
         commandArgs,
-        environment: this.environment,
+        environment: resolvedEnvironment,
         environmentVariables: {},
         withQualityGate,
       });
@@ -409,7 +424,7 @@ export class RunCommand extends Command {
           cwd,
           command,
           commandArgs,
-          environment: this.environment,
+          environment: resolvedEnvironment,
           environmentVariables: {
             ALLURE_TESTPLAN_PATH: testPlanPath,
             ALLURE_RERUN: `${rerun}`,
@@ -429,7 +444,7 @@ export class RunCommand extends Command {
         const { results } = await allureReport.validate({
           trs,
           knownIssues,
-          environment: this.environment,
+          environment: resolvedEnvironment,
         });
 
         qualityGateResults = results;

packages/cli/test/commands/qualityGate.test.ts

@@ -2,7 +2,7 @@ import * as console from "node:console";
 import { exit } from "node:process";
 
 import { readConfig, stringifyQualityGateResults } from "@allurereport/core";
-import { run } from "clipanion";
+import { UsageError, run } from "clipanion";
 import { glob } from "glob";
 import { type Mock, beforeEach, describe, expect, it, vi } from "vitest";
 
@@ -244,4 +244,13 @@ describe("quality-gate command", () => {
       knownIssuesPath: undefined,
     });
   });
+
+  it("should fail with usage error for invalid --environment value", async () => {
+    const command = new QualityGateCommand();
+
+    command.environment = "foo\nbar";
+
+    await expect(command.execute()).rejects.toBeInstanceOf(UsageError);
+    expect(readConfig).not.toHaveBeenCalled();
+  });
 });

packages/cli/test/commands/run.test.ts

@@ -0,0 +1,23 @@
+import { UsageError } from "clipanion";
+import { describe, expect, it } from "vitest";
+
+import { RunCommand } from "../../src/commands/run.js";
+
+describe("run command", () => {
+  it("should fail with usage error when command to run is missing", async () => {
+    const command = new RunCommand();
+
+    command.commandToRun = [];
+
+    await expect(command.execute()).rejects.toBeInstanceOf(UsageError);
+  });
+
+  it("should fail with usage error for newline in --environment", async () => {
+    const command = new RunCommand();
+
+    command.commandToRun = ["--", "echo", "hi"];
+    command.environment = "foo\nbar";
+
+    await expect(command.execute()).rejects.toBeInstanceOf(UsageError);
+  });
+});

packages/core-api/src/utils/environment.ts

@@ -2,6 +2,65 @@ import type { EnvironmentsConfig } from "../environment.js";
 import type { TestEnvGroup, TestResult } from "../model.js";
 
 export const DEFAULT_ENVIRONMENT = "default";
+export const MAX_ENVIRONMENT_NAME_LENGTH = 64;
+
+const hasControlChars = (value: string): boolean => {
+  for (let i = 0; i < value.length; i++) {
+    const code = value.charCodeAt(i);
+
+    // Reject ASCII control ranges: C0 (U+0000..U+001F), DEL (U+007F), and C1 (U+0080..U+009F).
+    // Common examples: \u0000 (NUL), \t (TAB), \n (LF), \r (CR), \u009F.
+    if (code <= 0x1f || (code >= 0x7f && code <= 0x9f)) {
+      return true;
+    }
+  }
+
+  return false;
+};
+
+export type EnvironmentValidationResult = { valid: true; normalized: string } | { valid: false; reason: string };
+
+export const validateEnvironmentName = (name: unknown): EnvironmentValidationResult => {
+  if (typeof name !== "string") {
+    return { valid: false, reason: "name must be a string" };
+  }
+
+  const normalized = name.trim();
+
+  if (normalized.length === 0) {
+    return { valid: false, reason: "name must not be empty" };
+  }
+
+  if (normalized.length > MAX_ENVIRONMENT_NAME_LENGTH) {
+    return {
+      valid: false,
+      reason: `name must not exceed ${MAX_ENVIRONMENT_NAME_LENGTH} characters`,
+    };
+  }
+
+  if (hasControlChars(normalized)) {
+    return { valid: false, reason: "name must not contain control characters" };
+  }
+
+  return { valid: true, normalized };
+};
+
+export const assertValidEnvironmentName = (name: unknown, source: string = "environment name"): string => {
+  const validationResult = validateEnvironmentName(name);
+
+  if (!validationResult.valid) {
+    throw new Error(`Invalid ${source} ${JSON.stringify(name)}: ${validationResult.reason}`);
+  }
+
+  return validationResult.normalized;
+};
+
+export const formatNormalizedEnvironmentCollision = (
+  sourcePath: string,
+  normalized: string,
+  originalKeys: string[],
+): string =>
+  `${sourcePath}: normalized key ${JSON.stringify(normalized)} is produced by original keys [${originalKeys.map((key) => JSON.stringify(key)).join(",")}]`;
 
 export const matchEnvironment = (envConfig: EnvironmentsConfig, tr: Pick<TestResult, "labels">): string => {
   return (

packages/core-api/test/utils/environment.test.ts

@@ -2,7 +2,13 @@ import { describe, expect, it } from "vitest";
 
 import type { EnvironmentsConfig } from "../../src/index.js";
 import type { TestResult } from "../../src/model.js";
-import { matchEnvironment } from "../../src/utils/environment.js";
+import {
+  MAX_ENVIRONMENT_NAME_LENGTH,
+  assertValidEnvironmentName,
+  formatNormalizedEnvironmentCollision,
+  matchEnvironment,
+  validateEnvironmentName,
+} from "../../src/utils/environment.js";
 
 const fixtures = {
   envConfig: {
@@ -42,3 +48,99 @@ describe("matchEnvironment", () => {
     expect(result).toEqual("default");
   });
 });
+
+describe("validateEnvironmentName", () => {
+  it("accepts valid names and returns normalized value", () => {
+    const validBoundaryName = "a".repeat(MAX_ENVIRONMENT_NAME_LENGTH);
+
+    expect(validateEnvironmentName("foo")).toEqual({ valid: true, normalized: "foo" });
+    expect(validateEnvironmentName("__proto__")).toEqual({ valid: true, normalized: "__proto__" });
+    expect(validateEnvironmentName("прод")).toEqual({ valid: true, normalized: "прод" });
+    expect(validateEnvironmentName(validBoundaryName)).toEqual({ valid: true, normalized: validBoundaryName });
+    expect(validateEnvironmentName("  foo  ")).toEqual({ valid: true, normalized: "foo" });
+    expect(validateEnvironmentName(" default ")).toEqual({ valid: true, normalized: "default" });
+  });
+
+  it("accepts names previously blocked by filesystem-style checks", () => {
+    expect(validateEnvironmentName("foo/bar")).toEqual({ valid: true, normalized: "foo/bar" });
+    expect(validateEnvironmentName("foo#bar")).toEqual({ valid: true, normalized: "foo#bar" });
+    expect(validateEnvironmentName("foo%bar")).toEqual({ valid: true, normalized: "foo%bar" });
+    expect(validateEnvironmentName("foo:bar")).toEqual({ valid: true, normalized: "foo:bar" });
+    expect(validateEnvironmentName(".")).toEqual({ valid: true, normalized: "." });
+    expect(validateEnvironmentName("..")).toEqual({ valid: true, normalized: ".." });
+  });
+
+  it("rejects empty and whitespace-only names", () => {
+    expect(validateEnvironmentName("")).toEqual({
+      valid: false,
+      reason: "name must not be empty",
+    });
+    expect(validateEnvironmentName("   ")).toEqual({
+      valid: false,
+      reason: "name must not be empty",
+    });
+  });
+
+  it("rejects too long names after trim", () => {
+    const tooLongName = ` ${"a".repeat(MAX_ENVIRONMENT_NAME_LENGTH + 1)} `;
+
+    expect(validateEnvironmentName(tooLongName)).toEqual({
+      valid: false,
+      reason: `name must not exceed ${MAX_ENVIRONMENT_NAME_LENGTH} characters`,
+    });
+  });
+
+  it("rejects control characters", () => {
+    expect(validateEnvironmentName("foo\nbar")).toEqual({
+      valid: false,
+      reason: "name must not contain control characters",
+    });
+    expect(validateEnvironmentName("foo\tbar")).toEqual({
+      valid: false,
+      reason: "name must not contain control characters",
+    });
+    expect(validateEnvironmentName("foo\u0000bar")).toEqual({
+      valid: false,
+      reason: "name must not contain control characters",
+    });
+    expect(validateEnvironmentName("foo\u009Fbar")).toEqual({
+      valid: false,
+      reason: "name must not contain control characters",
+    });
+    expect(validateEnvironmentName("foo\rbar")).toEqual({
+      valid: false,
+      reason: "name must not contain control characters",
+    });
+    expect(validateEnvironmentName("foo\r\nbar")).toEqual({
+      valid: false,
+      reason: "name must not contain control characters",
+    });
+  });
+
+  it("rejects non-string input", () => {
+    expect(validateEnvironmentName(1)).toEqual({
+      valid: false,
+      reason: "name must be a string",
+    });
+  });
+});
+
+describe("assertValidEnvironmentName", () => {
+  it("returns normalized value", () => {
+    expect(assertValidEnvironmentName("  foo  ")).toBe("foo");
+  });
+
+  it("throws with source details", () => {
+    expect(() => assertValidEnvironmentName("", "config.environment")).toThrow(
+      'Invalid config.environment "": name must not be empty',
+    );
+  });
+});
+
+describe("formatNormalizedEnvironmentCollision", () => {
+  it("formats stable collision message", () => {
+    expect(formatNormalizedEnvironmentCollision("config.environments", "foo", ["foo", " foo "])).toBe(
+      'config.environments: normalized key "foo" is produced by original keys ["foo"," foo "]',
+    );
+  });
+});