Merge branch 'main' into henrymercer/overlay-repo-property

src/feature-flags.ts

@@ -47,7 +47,6 @@ export enum Feature {
   ExportDiagnosticsEnabled = "export_diagnostics_enabled",
   ForceNightly = "force_nightly",
   IgnoreGeneratedFiles = "ignore_generated_files",
-  ImprovedProxyCertificates = "improved_proxy_certificates",
   JavaNetworkDebugging = "java_network_debugging",
   OverlayAnalysis = "overlay_analysis",
   OverlayAnalysisActions = "overlay_analysis_actions",
@@ -177,11 +176,6 @@ export const featureConfig = {
     envVar: "CODEQL_ACTION_IGNORE_GENERATED_FILES",
     minimumVersion: undefined,
   },
-  [Feature.ImprovedProxyCertificates]: {
-    defaultValue: false,
-    envVar: "CODEQL_ACTION_IMPROVED_PROXY_CERTIFICATES",
-    minimumVersion: undefined,
-  },
   [Feature.JavaNetworkDebugging]: {
     defaultValue: false,
     envVar: "CODEQL_ACTION_JAVA_NETWORK_DEBUGGING",

src/init-action.ts

@@ -96,6 +96,8 @@ import {
   GitHubVersion,
   Result,
   getOptionalEnvVar,
+  Success,
+  Failure,
 } from "./util";
 import { checkWorkflow } from "./workflow";
 
@@ -834,25 +836,25 @@ async function loadRepositoryProperties(
       "Skipping loading repository properties because the repository is owned by a user and " +
         "therefore cannot have repository properties.",
     );
-    return Result.success({});
+    return new Success({});
   }
 
   if (!(await features.getValue(Feature.UseRepositoryProperties))) {
     logger.debug(
       "Skipping loading repository properties because the UseRepositoryProperties feature flag is disabled.",
     );
-    return Result.success({});
+    return new Success({});
   }
 
   try {
-    return Result.success(
+    return new Success(
       await loadPropertiesFromApi(gitHubVersion, logger, repositoryNwo),
     );
   } catch (error) {
     logger.warning(
       `Failed to load repository properties: ${getErrorMessage(error)}`,
     );
-    return Result.failure(error);
+    return new Failure(error);
   }
 }
 

src/start-proxy-action.ts

@@ -90,9 +90,7 @@ async function run(startedAt: Date) {
       }
     }
 
-    const ca = generateCertificateAuthority(
-      await features.getValue(Feature.ImprovedProxyCertificates),
-    );
+    const ca = generateCertificateAuthority();
 
     const proxyConfig: ProxyConfig = {
       all_credentials: credentials,

src/start-proxy/ca.test.ts

@@ -32,33 +32,7 @@ function checkCertAttributes(
 }
 
 test("generateCertificateAuthority - generates certificates", (t) => {
-  const result = ca.generateCertificateAuthority(false);
-  const cert = pki.certificateFromPem(result.cert);
-  const key = pki.privateKeyFromPem(result.key);
-
-  t.truthy(cert);
-  t.truthy(key);
-
-  checkCertAttributes(t, cert);
-
-  // Check the validity.
-  t.true(
-    cert.validity.notBefore <= new Date(),
-    "notBefore date is in the future",
-  );
-  t.true(cert.validity.notAfter > new Date(), "notAfter date is in the past");
-
-  // Check that the extensions are set as we'd expect.
-  const exts = cert.extensions as ca.Extension[];
-  t.is(exts.length, 1);
-  t.is(exts[0].name, "basicConstraints");
-  t.is(exts[0].cA, true);
-
-  t.truthy(cert.siginfo);
-});
-
-test("generateCertificateAuthority - generates certificates with FF", (t) => {
-  const result = ca.generateCertificateAuthority(true);
+  const result = ca.generateCertificateAuthority();
   const cert = pki.certificateFromPem(result.cert);
   const key = pki.privateKeyFromPem(result.key);
 

src/start-proxy/ca.ts

@@ -37,7 +37,8 @@ export type Extension = {
   [key: string]: unknown;
 };
 
-const extraExtensions: Extension[] = [
+const allExtensions: Extension[] = [
+  { name: "basicConstraints", cA: true },
   {
     name: "keyUsage",
     critical: true,
@@ -52,12 +53,9 @@ const extraExtensions: Extension[] = [
 /**
  * Generates a CA certificate for the proxy.
  *
- * @param newCertGenFF Whether to use the updated certificate generation.
  * @returns The private and public keys.
  */
-export function generateCertificateAuthority(
-  newCertGenFF: boolean,
-): CertificateAuthority {
+export function generateCertificateAuthority(): CertificateAuthority {
   const keys = pki.rsa.generateKeyPair(KEY_SIZE);
   const cert = pki.createCertificate();
   cert.publicKey = keys.publicKey;
@@ -71,21 +69,11 @@ export function generateCertificateAuthority(
   cert.setSubject(CERT_SUBJECT);
   cert.setIssuer(CERT_SUBJECT);
 
-  const extensions: Extension[] = [{ name: "basicConstraints", cA: true }];
+  // Set the CA extensions for the certificate.
+  cert.setExtensions(allExtensions);
 
-  // Add the extra CA extensions if the FF is enabled.
-  if (newCertGenFF) {
-    extensions.push(...extraExtensions);
-  }
-
-  cert.setExtensions(extensions);
-
-  // Specifically use SHA256 when the FF is enabled.
-  if (newCertGenFF) {
-    cert.sign(keys.privateKey, md.sha256.create());
-  } else {
-    cert.sign(keys.privateKey);
-  }
+  // Specifically use SHA256 to ensure consistency and compatibility.
+  cert.sign(keys.privateKey, md.sha256.create());
 
   const pem = pki.certificateToPem(cert);
   const key = pki.privateKeyToPem(keys.privateKey);

src/util.test.ts

@@ -564,27 +564,27 @@ test("joinAtMost - truncates list if array is > than limit", (t) => {
   t.false(result.includes("test6"));
 });
 
-test("Result.success creates a success result", (t) => {
-  const result = util.Result.success("test value");
+test("Success creates a success result", (t) => {
+  const result = new util.Success("test value");
   t.true(result.isSuccess());
   t.false(result.isFailure());
   t.is(result.value, "test value");
 });
 
-test("Result.failure creates a failure result", (t) => {
+test("Failure creates a failure result", (t) => {
   const error = new Error("test error");
-  const result = util.Result.failure(error);
+  const result = new util.Failure(error);
   t.false(result.isSuccess());
   t.true(result.isFailure());
   t.is(result.value, error);
 });
 
-test("Result.orElse returns the value for a success result", (t) => {
-  const result = util.Result.success("success value");
+test("Success.orElse returns the value for a success result", (t) => {
+  const result = new util.Success("success value");
   t.is(result.orElse("default value"), "success value");
 });
 
-test("Result.orElse returns the default value for a failure result", (t) => {
-  const result = util.Result.failure(new Error("test error"));
+test("Failure.orElse returns the default value for a failure result", (t) => {
+  const result = new util.Failure(new Error("test error"));
   t.is(result.orElse("default value"), "default value");
 });

src/util.ts

@@ -1292,42 +1292,51 @@ export function joinAtMost(
   return array.join(separator);
 }
 
-/** A success result. */
-type Success<T> = Result<T, never>;
-/** A failure result. */
-type Failure<E> = Result<never, E>;
-
-/**
- * A simple result type representing either a success or a failure.
- */
-export class Result<T, E> {
-  private constructor(
-    private readonly _ok: boolean,
-    public readonly value: T | E,
-  ) {}
-
-  /** Creates a success result. */
-  static success<T>(value: T): Success<T> {
-    return new Result(true, value) as Success<T>;
-  }
-
-  /** Creates a failure result. */
-  static failure<E>(value: E): Failure<E> {
-    return new Result(false, value) as Failure<E>;
-  }
-
+/** An interface representing something that is either a success or a failure. */
+interface ResultLike<T, E> {
+  /** The value of the result, which can be either a success value or a failure value. */
+  value: T | E;
   /** Whether this result represents a success. */
-  isSuccess(): this is Success<T> {
-    return this._ok;
-  }
-
+  isSuccess(): this is Success<T>;
   /** Whether this result represents a failure. */
-  isFailure(): this is Failure<E> {
-    return !this._ok;
+  isFailure(): this is Failure<E>;
+  /** Get the value if this is a success, or return the default value if this is a failure. */
+  orElse<U>(defaultValue: U): T | U;
+}
+
+/** A simple result type representing either a success or a failure. */
+export type Result<T, E> = Success<T> | Failure<E>;
+
+/** A result representing a success. */
+export class Success<T> implements ResultLike<T, never> {
+  constructor(public readonly value: T) {}
+
+  isSuccess(): this is Success<T> {
+    return true;
   }
 
-  /** Get the value if this is a success, or return the default value if this is a failure. */
-  orElse<U>(defaultValue: U): T | U {
-    return this.isSuccess() ? this.value : defaultValue;
+  isFailure(): this is Failure<never> {
+    return false;
+  }
+
+  orElse<U>(_defaultValue: U): T {
+    return this.value;
+  }
+}
+
+/** A result representing a failure. */
+export class Failure<E> implements ResultLike<never, E> {
+  constructor(public readonly value: E) {}
+
+  isSuccess(): this is Success<never> {
+    return false;
+  }
+
+  isFailure(): this is Failure<E> {
+    return true;
+  }
+
+  orElse<U>(defaultValue: U): U {
+    return defaultValue;
   }
 }