fix: preserve KNOWBE4_BASE_URL path prefix when building request URLs

[asachs01]

Root cause

src/utils/client.ts built every outbound request URL with:

const url = new URL(path, creds.baseUrl);

The two-argument URL constructor performs RFC 3986 relative resolution, not string concatenation. Every call site in this repo passes a path-absolute reference ("/api/v1/account", "/api/v1/phishing/security_tests", …), and by that rule a path-absolute reference replaces the base URL's path component entirely.

So with KNOWBE4_BASE_URL=https://proxy.corp.example/knowbe4/:

expected	https://proxy.corp.example/knowbe4/api/v1/account
actual	https://proxy.corp.example/api/v1/account

The /knowbe4 prefix is gone and every request goes to the wrong place — silently, with no error.

Why it's invisible by default

The built-in region defaults (https://us.api.knowbe4.com, eu., ca., uk., de.) are bare origins with no path, so there is nothing for relative resolution to discard. Every existing test and every default deployment is unaffected, which is exactly why this survived.

It only bites self-hosters who set KNOWBE4_BASE_URL to a reverse proxy or API gateway mounted under a path — the case the README documents as "Custom base URL (overrides region)".

The fix

Explicit slash normalization plus concatenation, so the base's path survives regardless of leading/trailing slashes on either side:

// Join by string, not `new URL(path, base)`: the two-arg form does RFC 3986
// relative resolution, so a path-absolute reference like "/api/v1/account"
// replaces the base's own path entirely. That silently drops the prefix of a
// KNOWBE4_BASE_URL pointing at a proxy/gateway (e.g. https://proxy/knowbe4/).
const normalizedBase = creds.baseUrl.replace(/\/+$/, "");
const normalizedPath = path.replace(/^\/+/, "");
const url = new URL(`${normalizedBase}/${normalizedPath}`);

The comment is there on purpose — this reads like a line begging to be "simplified" back.

Regression tests

New describe("apiRequest URL construction") block in src/__tests__/client.test.ts. It stubs globalThis.fetch via vi.stubGlobal, calls the real exported apiRequest, and asserts the exact URL string fetch was handed. Coverage:

• bare-origin base (region default) — unchanged behaviour
• base with a path prefix and a trailing slash — prefix survives
• base with a path prefix and no trailing slash — prefix survives
• redundant/doubled slashes on either side collapse to exactly one
• query params still append after the joined path

Env vars are set in beforeEach and restored in afterEach alongside vi.unstubAllGlobals().

Evidence the test actually catches the bug

The fix was temporarily reverted to the original one-liner and the suite re-run:

 FAIL  src/__tests__/client.test.ts > apiRequest URL construction > should preserve a base URL path prefix with a trailing slash
 FAIL  src/__tests__/client.test.ts > apiRequest URL construction > should preserve a base URL path prefix without a trailing slash
 FAIL  src/__tests__/client.test.ts > apiRequest URL construction > should collapse redundant slashes on either side of the join
 FAIL  src/__tests__/client.test.ts > apiRequest URL construction > should append query params after the joined path

 Test Files  1 failed | 4 passed (5)
      Tests  4 failed | 60 passed (64)

All four path-prefix cases fail against the pre-fix code. The bare-origin case passes both before and after, as expected — that's the whole reason the bug went unnoticed.

One case is worse than "prefix dropped": new URL("//api/v1/users", "https://proxy.corp.example/knowbe4//") resolves to https://api/v1/users. A leading // is a protocol-relative reference, so it replaces the host too, not just the path.

With the fix restored: 64 passed (64).

Verification

npm run build   → exit 0 (tsc clean)
npm run lint    → exit 0 — 5 problems (0 errors, 5 warnings)
npm test        → Test Files 5 passed (5) | Tests 64 passed (64)

The 5 lint warnings are pre-existing unused-import warnings in src/domains/{phishing,training,users}.ts, untouched by this change. Zero errors.

Scope

Swept src/ for other two-arg new URL( calls. The only other instance is src/index.ts:627:

const url = new URL(req.url || "/", `http://${req.headers.host || "localhost"}`);

That's inbound request parsing where relative resolution is the correct and intended behaviour. Left alone.