W-19264912 feat: add domain name validation for outgoing requests (#218)

* add domain name validation to outgoing requests

* reword

* fix domain check to include localhost

* use ux.warn for warning message

* Apply suggestions from code review

Co-authored-by: Justin Wilaby <jwilaby@gmail.com>
Signed-off-by: Eric <eablack@gmail.com>

* remove unnecessary as const

---------

Signed-off-by: Eric <eablack@gmail.com>
Co-authored-by: Justin Wilaby <jwilaby@gmail.com>

Author: eablack

src/api-client.ts

@@ -14,6 +14,9 @@ import {yubikey} from './yubikey.js'
 
 const netrc = new Netrc()
 
+export const ALLOWED_HEROKU_DOMAINS = Object.freeze(['heroku.com', 'herokai.com', 'herokuspace.com', 'herokudev.com'])
+export const LOCALHOST_DOMAINS = Object.freeze(['localhost', '127.0.0.1'])
+
 // eslint-disable-next-line @typescript-eslint/no-namespace
 export namespace APIClient {
   export interface Options extends HTTPRequestOptions {
@@ -137,6 +140,7 @@ export class APIClient {
         delinquencyConfig.warning_shown = true
       }
 
+      // eslint-disable-next-line complexity
       static async request<T>(url: string, opts: APIClient.Options = {}, retries = 3): Promise<APIHTTPClient<T>> {
         opts.headers = opts.headers || {}
         const currentRequestId = RequestId.create() && RequestId.headerValue
@@ -155,7 +159,22 @@ export class APIClient {
         }
 
         if (!Object.keys(opts.headers).some(h => h.toLowerCase() === 'authorization')) {
-          opts.headers.authorization = `Bearer ${self.auth}`
+          // Handle both relative and absolute URLs for validation
+          let targetUrl: URL
+          try {
+            // Try absolute URL first
+            targetUrl = new URL(url)
+          } catch {
+            // If that fails, assume it's relative and prepend the API base URL
+            targetUrl = new URL(url, vars.apiUrl)
+          }
+
+          const isHerokuApi = ALLOWED_HEROKU_DOMAINS.some(domain => targetUrl.hostname.endsWith(`.${domain}`))
+          const isLocalhost = LOCALHOST_DOMAINS.includes(targetUrl.hostname as (typeof LOCALHOST_DOMAINS)[number])
+
+          if (isHerokuApi || isLocalhost) {
+            opts.headers.authorization = `Bearer ${self.auth}`
+          }
         }
 
         this.configDelinquency(url, opts)

src/vars.ts

@@ -1,5 +1,8 @@
+import {ux} from '@oclif/core'
 import * as url from 'node:url'
 
+import {ALLOWED_HEROKU_DOMAINS, LOCALHOST_DOMAINS} from './api-client.js'
+
 export class Vars {
   get apiHost(): string {
     if (this.host.startsWith('http')) {
@@ -41,7 +44,14 @@ export class Vars {
   }
 
   get host(): string {
-    return this.envHost || 'heroku.com'
+    const {envHost} = this
+
+    if (envHost && !this.isValidHerokuHost(envHost)) {
+      ux.warn(`Invalid HEROKU_HOST '${envHost}' - using default`)
+      return 'heroku.com'
+    }
+
+    return envHost || 'heroku.com'
   }
 
   get httpGitHost(): string {
@@ -62,6 +72,13 @@ export class Vars {
       ? 'https://particleboard-staging-cloud.herokuapp.com'
       : 'https://particleboard.heroku.com'
   }
+
+  private isValidHerokuHost(host: string): boolean {
+    // Remove protocol if present
+    const cleanHost = host.replace(/^https?:\/\//, '')
+
+    return ALLOWED_HEROKU_DOMAINS.some(domain => cleanHost.endsWith(`.${domain}`)) || LOCALHOST_DOMAINS.some(domain => cleanHost.includes(domain))
+  }
 }
 
 export const vars = new Vars()

test/api-client.test.ts

@@ -112,6 +112,16 @@ describe('api_client', () => {
   })
 
   describe('with HEROKU_HOST', () => {
+    test
+      .it('rejects invalid HEROKU_HOST and uses default API', async ctx => {
+        process.env.HEROKU_HOST = 'http://bogus-server.com'
+        api = nock('https://api.heroku.com') // Should fallback to default
+        api.get('/apps').reply(200, [{name: 'myapp'}])
+
+        const cmd = new Command([], ctx.config)
+        await cmd.heroku.get('/apps')
+      })
+
     test
       .it('makes an HTTP request with HEROKU_HOST', async ctx => {
         const localHostURI = 'http://localhost:5000'

test/vars.test.ts

@@ -22,28 +22,46 @@ describe('vars', () => {
     expect(vars.particleboardUrl).to.equal('https://particleboard.heroku.com')
   })
 
-  it('respects HEROKU_HOST', () => {
-    process.env.HEROKU_HOST = 'customhost'
-    expect(vars.apiHost).to.equal('api.customhost')
-    expect(vars.apiUrl).to.equal('https://api.customhost')
-    expect(vars.gitHost).to.equal('customhost')
-    expect(vars.host).to.equal('customhost')
-    expect(vars.httpGitHost).to.equal('git.customhost')
-    expect(vars.gitPrefixes).to.deep.equal(['git@customhost:', 'ssh://git@customhost/', 'https://git.customhost/'])
+  it('respects valid HEROKU_HOST values', () => {
+    // Test with a valid heroku.com subdomain
+    process.env.HEROKU_HOST = 'staging.heroku.com'
+    expect(vars.apiHost).to.equal('api.staging.heroku.com')
+    expect(vars.apiUrl).to.equal('https://api.staging.heroku.com')
+    expect(vars.gitHost).to.equal('staging.heroku.com')
+    expect(vars.host).to.equal('staging.heroku.com')
+    expect(vars.httpGitHost).to.equal('git.staging.heroku.com')
+    expect(vars.gitPrefixes).to.deep.equal(['git@staging.heroku.com:', 'ssh://git@staging.heroku.com/', 'https://git.staging.heroku.com/'])
     expect(vars.particleboardUrl).to.equal('https://particleboard.heroku.com')
   })
 
-  it('respects HEROKU_HOST as url', () => {
-    process.env.HEROKU_HOST = 'https://customhost'
-    expect(vars.host).to.equal('https://customhost')
-    expect(vars.apiHost).to.equal('customhost')
-    expect(vars.apiUrl).to.equal('https://customhost')
-    expect(vars.gitHost).to.equal('customhost')
-    expect(vars.httpGitHost).to.equal('customhost')
-    expect(vars.gitPrefixes).to.deep.equal(['git@customhost:', 'ssh://git@customhost/', 'https://customhost/'])
+  it('rejects invalid HEROKU_HOST values for security', () => {
+    // Test that invalid hosts are rejected and fallback to default
+    process.env.HEROKU_HOST = 'bogus-server.com'
+    expect(vars.host).to.equal('heroku.com') // Should fallback to default
+    expect(vars.apiHost).to.equal('api.heroku.com')
+    expect(vars.apiUrl).to.equal('https://api.heroku.com')
+  })
+
+  it('respects legitimate HEROKU_HOST as url', () => {
+    // Test with a valid heroku.com subdomain URL
+    process.env.HEROKU_HOST = 'https://staging.heroku.com'
+    expect(vars.host).to.equal('https://staging.heroku.com')
+    expect(vars.apiHost).to.equal('staging.heroku.com')
+    expect(vars.apiUrl).to.equal('https://staging.heroku.com')
+    expect(vars.gitHost).to.equal('staging.heroku.com')
+    expect(vars.httpGitHost).to.equal('staging.heroku.com')
+    expect(vars.gitPrefixes).to.deep.equal(['git@staging.heroku.com:', 'ssh://git@staging.heroku.com/', 'https://staging.heroku.com/'])
     expect(vars.particleboardUrl).to.equal('https://particleboard.heroku.com')
   })
 
+  it('rejects invalid HEROKU_HOST URLs', () => {
+    // Test that invalid URL hosts are rejected and fallback to default
+    process.env.HEROKU_HOST = 'https://bogus-server.com'
+    expect(vars.host).to.equal('heroku.com') // Should fallback to default for security
+    expect(vars.apiHost).to.equal('api.heroku.com')
+    expect(vars.apiUrl).to.equal('https://api.heroku.com')
+  })
+
   it('respects HEROKU_PARTICLEBOARD_URL', () => {
     process.env.HEROKU_PARTICLEBOARD_URL = 'https://customhost'
     expect(vars.particleboardUrl).to.equal('https://customhost')