Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions packages/cli/src/lib/db/regions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,33 @@ export function getOrgRegion(
| Pick<OrgRegionRow, "region_url">
| undefined;

recordCacheHit("region", !!row);
return row?.region_url;
const regionUrl = usableRegionUrl(row?.region_url, orgSlug);
recordCacheHit("region", !!regionUrl);
return regionUrl;
}

/**
* Reject cached region URLs that are not absolute HTTP(S) URLs.
*
* Older CLI releases persisted a self-hosted org's raw `links.regionUrl`
* (e.g. "" or "/"). Returning such a value makes SDK requests use a bare
* path like `/api/0/...`, which `new Request()` rejects (CLI-2DE). Treating
* it as a cache miss forces a fresh lookup that resolves an absolute URL.
*/
function usableRegionUrl(
regionUrl: string | undefined,
orgSlug: string,
): string | undefined {
if (!regionUrl) {
return;
}
if (!normalizeHttpOrigin(regionUrl)) {
log.debug(
`Ignoring cached non-absolute region URL for '${orgSlug}': '${regionUrl}'`,
);
return;
}
return regionUrl;
}

/**
Expand Down Expand Up @@ -299,10 +324,11 @@ export function getOrgByNumericId(
| Pick<OrgRegionRow, "org_slug" | "region_url">
| undefined;

if (!row) {
const regionUrl = usableRegionUrl(row?.region_url, row?.org_slug ?? "");
if (!(row && regionUrl)) {
return;
}
return { slug: row.org_slug, regionUrl: row.region_url };
return { slug: row.org_slug, regionUrl };
}

/**
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/lib/sentry-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
} from "./db/auth.js";
import {
ApiError,
ConfigError,
HostScopeError,
MalformedAuthTokenError,
TimeoutError,
Expand Down Expand Up @@ -1118,6 +1119,15 @@ export function getSdkConfig(
? regionUrl.slice(0, -1)
: regionUrl;

// A relative/empty base makes SDK requests use a bare `/api/0/...` path,
// which undici rejects with an opaque "Failed to parse URL" (CLI-2DE).
if (!normalizeHttpOrigin(normalizedBase)) {
throw new ConfigError(
`Invalid Sentry API base URL: '${regionUrl}' is not an absolute HTTP(S) URL.`,
"Set SENTRY_URL to your Sentry instance (e.g. https://sentry.example.com) and retry.",
);
}

return {
// SDK functions already include /api/0/ in their URL paths,
// so baseUrl should be the plain region URL without /api/0.
Expand Down
15 changes: 15 additions & 0 deletions packages/cli/test/lib/db/regions.focused.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, expect, test } from "vitest";
import { getDatabase } from "../../../src/lib/db/index.js";
import {
getOrgByNumericId,
getOrgRegion,
isTrustedRegionOrigin,
resetTrustedRegionUrlsForTesting,
Expand Down Expand Up @@ -62,4 +64,17 @@ describe("credential and origin scoped organization regions", () => {
setOrgRegion("invalid", regionUrl, SOURCE_A, SOURCE_A, IDENTITY_A),
).toThrow();
});

test.each(["", "/", "/sentry"])(
"treats a legacy non-absolute cached region URL %j as a cache miss (CLI-2DE)",
(regionUrl) => {
getDatabase()
.query(
"INSERT INTO org_regions (org_slug, org_id, region_url, credential_identity, source_origin, response_origin, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?)",
)
.run("legacy", "4242", regionUrl, IDENTITY_A, SOURCE_A, SOURCE_A, 0);
expect(getOrgRegion("legacy", SOURCE_A, IDENTITY_A)).toBeUndefined();
expect(getOrgByNumericId("4242", SOURCE_A, IDENTITY_A)).toBeUndefined();
},
);
});
34 changes: 34 additions & 0 deletions packages/cli/test/lib/region.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,40 @@ describe("resolveOrgRegion", () => {
}
});

test("falls back to baseUrl when API returns an empty regionUrl (CLI-2DE)", async () => {
const originalFetch = globalThis.fetch;
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
const req = new Request(input, init);
if (req.url.includes("/organizations/empty-region-org/")) {
return new Response(
JSON.stringify({
id: "791",
slug: "empty-region-org",
name: "Self-hosted Org 3",
links: {
organizationUrl: "/organizations/empty-region-org/",
regionUrl: "",
},
}),
{
status: 200,
headers: { "Content-Type": "application/json" },
},
);
}
return new Response(JSON.stringify({ detail: "Not found" }), {
status: 404,
});
};

try {
const regionUrl = await resolveOrgRegion("empty-region-org");
expect(regionUrl).toBe("https://sentry.io");
} finally {
globalThis.fetch = originalFetch;
}
});

test("falls back to baseUrl when API returns a malformed regionUrl", async () => {
const originalFetch = globalThis.fetch;
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
Expand Down
15 changes: 14 additions & 1 deletion packages/cli/test/lib/sentry-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { afterEach, beforeEach, describe, expect, test } from "vitest";
import { setAuthToken } from "../../src/lib/db/auth.js";
import { TimeoutError } from "../../src/lib/errors.js";
import { ConfigError, TimeoutError } from "../../src/lib/errors.js";
import {
__injectTimeoutOverrideForTests,
__resolveRequestTimeoutMsForTests,
Expand Down Expand Up @@ -35,6 +35,19 @@ function getAuthenticatedFetch(): typeof fetch {
return getSdkConfig(REGION_URL).fetch as typeof fetch;
}

describe("getSdkConfig base URL validation", () => {
test.each(["", "/", "/sentry/"])(
"rejects non-absolute base URL %j with ConfigError (CLI-2DE)",
(base) => {
expect(() => getSdkConfig(base)).toThrow(ConfigError);
},
);

test("strips a trailing slash from an absolute base URL", () => {
expect(getSdkConfig(`${REGION_URL}/`).baseUrl).toBe(REGION_URL);
});
});

describe("fetchWithRetry / buildAttemptFactory", () => {
test("retries a POST with a string body without re-consuming the body", async () => {
const marker = "__test_string_body__";
Expand Down
Loading