fix: generate default allowedHosts with available variables instead of skipping all due to a single missing one (#441)

* update deps, apply prettier, change to ESM, update tests

* update ci tests to use Node 22

* coderabbit 1

* remove exclude

* update lockfiles

* lint fixes

* fix prettier config

* fix npm ci

* coderabbit 2

* downgrade and pin angular

* update engine version range to match Angular

* fix version and readme

* update readme to suggest latest CLI

* add allowedHosts and trustedProxyHeaders

* regenrate lockfiles

* fix type for getContext

* update readme for deploy instructions

* prevent missing env vars from failing build

* fix for loop bug

* update swapped file templates

* fix context potentially being undefined

* consolidate app-engine exports in a single file

* conditionally update Angular config

* revert pathname to request

* fix typo

* fix merge conflicts

* match version of angular/ssr instead of core

* Update README.md

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>

* seperate version detection for angular/core and ssr

* update tests

* update failBuild message and remove dead code

* update readme

* allow specifying own hosts

* don't inject defaults when allowedHosts includes `*`

* allow trusting all proxy headers

* fix set logic

* fix type definition

* prettify

* remove unnecessary config options

---------

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>

Author: hrishikesh-k

src/app-engine.js

@@ -1,54 +1,142 @@
 import { env } from 'node:process'
 
+/**
+ * Generate `allowedHosts` config for `AngularAppEngine` from `@angular/ssr`
+ * @returns {string[]}
+ */
 export function getAllowedHosts() {
-  const allowedHosts = []
+  const defaultAllowedHosts = []
+
+  let deployId
+  let deployPrimeUrlHostname
+  let siteId
+  let siteName
+
   const environmentVariables = ['DEPLOY_ID', 'DEPLOY_PRIME_URL', 'DEPLOY_URL', 'SITE_ID', 'SITE_NAME', 'URL']
 
   for (const environmentVariable of environmentVariables) {
-    if (!env[environmentVariable] || env[environmentVariable] === 'undefined') {
-      console.warn(
-        `Missing Netlify-specific environment variable ${environmentVariable}. \`allowedHosts\` config might be incomplete.`,
-      )
-      return allowedHosts
+    switch (environmentVariable) {
+      case 'DEPLOY_ID':
+        // not setting this directly above so that validation
+        // and warnings remain centralized in the helper
+        deployId = getEnvironmentVariable(environmentVariable)
+        break
+      case 'DEPLOY_PRIME_URL':
+        // DEPLOY_PRIME_URL:
+        // <branch-name>--<subdomain>.netlify.app or
+        // <dp-#>--<subdomain>.netlify.app (supports ADS)
+        deployPrimeUrlHostname = getHostnameFromEnvironmentVariable(environmentVariable)
+        if (deployPrimeUrlHostname) {
+          defaultAllowedHosts.push(deployPrimeUrlHostname)
+        }
+        break
+      case 'DEPLOY_URL':
+      case 'URL': {
+        // DEPLOY_URL: <deploy-id>--<subdomain>.netlify.app
+        // URL: <subdomain>.netlify.app OR <custom-domain>
+        // www handling is not required as Netlify auto-redirects
+        const hostname = getHostnameFromEnvironmentVariable(environmentVariable)
+        if (hostname) {
+          defaultAllowedHosts.push(hostname)
+        }
+        break
+      }
+      case 'SITE_ID':
+        // SITE_ID: <site_id>
+        // this makes it <site_id>.netlify.app
+        // separate handling because we need it later
+        siteId = getEnvironmentVariable(environmentVariable)
+        if (siteId) {
+          defaultAllowedHosts.push(`${siteId}.netlify.app`)
+        }
+        break
+      case 'SITE_NAME':
+        // SITE_NAME: <subdomain>
+        // this makes it <subdomain>.netlify.app
+        // this may duplicate URL for sites without a custom domain
+        // but duplicates are removed later and this is still useful
+        // if a custom domain is removed after deployment
+        siteName = getEnvironmentVariable(environmentVariable)
+        if (siteName) {
+          defaultAllowedHosts.push(`${siteName}.netlify.app`)
+        }
+        break
+      default:
+        break
     }
   }
 
-  const deployPrimeUrlHostname = new URL(env.DEPLOY_PRIME_URL).hostname
-
-  // <subdomain>.netlify.app OR <custom-domain>
-  // www handling is not required as Netlify will auto-redirect
-  allowedHosts.push(new URL(env.URL).hostname)
-  // <deploy-id>--<subdomain>.netlify.app
-  allowedHosts.push(new URL(env.DEPLOY_URL).hostname)
-  // <branch-name>--<subdomain>.netlify.app or <dp-#>--<subdomain>.netlify.app (supports ADS)
-  allowedHosts.push(deployPrimeUrlHostname)
-  // <subdomain>.netlify.app
-  // this will be duplicated for sites without custom domain
-  // but it's important to have in case a site's custom domain is removed after a deploy
-  allowedHosts.push(`${env.SITE_NAME}.netlify.app`)
-  // <site-id>.netlify.app
-  allowedHosts.push(`${env.SITE_ID}.netlify.app`)
-  // <deploy-id>--<site-id>.netlify.app
-  allowedHosts.push(`${env.DEPLOY_ID}--${env.SITE_ID}.netlify.app`)
-
-  // we need to extract the branch name or the deploy preview number
-  // so we can add the subdomain as well as site-id specific URLs
-  // this would be required for sites using ADS so that
-  // we can add netlify.app URLs as well
-  if (deployPrimeUrlHostname.includes('--')) {
+  // extract the branch name or deploy preview number
+  // so we can generate ADS-compatible URLs
+  if (deployPrimeUrlHostname?.includes('--') && siteId && siteName) {
     const [branchNameOrDpNumber] = deployPrimeUrlHostname.split('--')
-    allowedHosts.push(`${branchNameOrDpNumber}--${env.SITE_NAME}.netlify.app`)
-    allowedHosts.push(`${branchNameOrDpNumber}--${env.SITE_ID}.netlify.app`)
+    defaultAllowedHosts.push(`${branchNameOrDpNumber}--${siteName}.netlify.app`)
+    defaultAllowedHosts.push(`${branchNameOrDpNumber}--${siteId}.netlify.app`)
+  }
+
+  if (deployId && siteId) {
+    // <deploy-id>--<site-id>.netlify.app
+    defaultAllowedHosts.push(`${deployId}--${siteId}.netlify.app`)
   }
 
-  return allowedHosts
+  return [...new Set(defaultAllowedHosts)]
 }
 
+/**
+ * Return Netlify-specific context
+ * @returns {import('@netlify/edge-functions').Context | undefined}
+ */
 export function getContext() {
   // eslint-disable-next-line no-undef
   return typeof Netlify !== 'undefined' ? Netlify?.context : undefined
 }
 
+/**
+ * Return the value of an environment variable
+ * @param {string} environmentVariable
+ *
+ * @returns {string | undefined}
+ */
+function getEnvironmentVariable(environmentVariable) {
+  const value = env[environmentVariable]
+
+  // we inject the literal string "undefined" for variables that don't have a value
+  if (value == null || value === '' || value === 'undefined') {
+    console.warn(
+      `Missing Netlify-specific environment variable ${environmentVariable}. ` +
+        '`allowedHosts` config might be incomplete.',
+    )
+
+    return
+  }
+
+  return value
+}
+
+/**
+ * Return hostname from the value of an environment variable
+ * @param {string} environmentVariable
+ *
+ * @returns {string | undefined}
+ */
+function getHostnameFromEnvironmentVariable(environmentVariable) {
+  const value = getEnvironmentVariable(environmentVariable)
+
+  if (value == null) {
+    return
+  }
+
+  try {
+    return new URL(value).hostname
+  } catch {
+    console.warn(`Netlify-specific environment variable ${environmentVariable} does not contain a valid URL`)
+  }
+}
+
+/**
+ * Generate `trustProxyHeaders` config for `AngularAppEngine` from `@angular/ssr`
+ * @returns {string[]}
+ */
 export function getTrustProxyHeaders() {
   return ['x-forwarded-for']
 }