Skip to content
Merged
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
37 changes: 27 additions & 10 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1293,25 +1293,42 @@ export function createApp() {
// Public-safe README status badge (#541). Unauthenticated and embeddable: it serves ONLY whitelisted,
// repo-level metrics, and ONLY for installed repos that opted in via the `badgeEnabled` setting. Excluded
// from requiresApiToken above; aggressively cached + stale-while-revalidate like the public stats route.
// #8377: loadPublicRepoBadge is NOT fail-safe (D1 reads plus a possible cold-cache GitHub manifest fetch),
// so a transient blip used to escape as Hono's bare 500 — unusually visible here, since these badges are
// embedded in third-party READMEs behind GitHub's camo proxy. Same route-level try/catch the sibling
// /quality route and the #4995 relay fix already use (the loader itself is untouched). 503, never 404:
// 404 stays reserved for the real "no public badge for this repo" case so a monitor can tell the two
// apart, and the 503 branch uses the SHORT cache so a transient failure is never cached for the long
// stale-while-revalidate window.
app.get("/v1/public/repos/:owner/:repo/badge.svg", async (c) => {
const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo"));
c.header("Content-Type", "image/svg+xml; charset=utf-8");
if (!quality) {
try {
const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo"));
if (!quality) {
c.header("Cache-Control", "public, max-age=300");
return c.body(renderUnavailableBadgeSvg(), 404);
}
c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400");
return c.body(renderBadgeSvg(quality));
} catch {
c.header("Cache-Control", "public, max-age=300");
return c.body(renderUnavailableBadgeSvg(), 404);
return c.body(renderUnavailableBadgeSvg(), 503);
}
c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400");
return c.body(renderBadgeSvg(quality));
});

app.get("/v1/public/repos/:owner/:repo/badge.json", async (c) => {
const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo"));
if (!quality) {
try {
const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo"));
if (!quality) {
c.header("Cache-Control", "public, max-age=300");
return c.json({ schemaVersion: 1, label: PUBLIC_BADGE_LABEL, message: "unavailable", color: "#9e9e9e", cacheSeconds: 300 }, 404);
}
c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400");
return c.json(buildShieldsBadge(quality, 600));
} catch {
c.header("Cache-Control", "public, max-age=300");
return c.json({ schemaVersion: 1, label: PUBLIC_BADGE_LABEL, message: "unavailable", color: "#9e9e9e", cacheSeconds: 300 }, 404);
return c.json({ schemaVersion: 1, label: PUBLIC_BADGE_LABEL, message: "unavailable", color: "#9e9e9e", cacheSeconds: 300 }, 503);
}
c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400");
return c.json(buildShieldsBadge(quality, 600));
});

// Public per-repo review-quality metrics (#2568). Unauthenticated; aggregate counts/rates only; opt-in via
Expand Down
39 changes: 39 additions & 0 deletions test/integration/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,27 @@ describe("api routes", () => {
const unknown = await app.request("/v1/public/repos/acme/missing/badge.json", {}, env);
expect(unknown.status).toBe(404);
await expect(unknown.json()).resolves.toMatchObject({ message: "unavailable" });

// REGRESSION (#8377): a backend failure inside loadPublicRepoBadge must render the graceful
// "unavailable" badge with 503 — not escape as Hono's bare, unstructured 500 (no app.onError exists).
// 503 (not 404) so a monitor can tell a transient backend problem apart from "this repo has no badge",
// and the short cache so a blip is never cached for the long stale-while-revalidate window.
const brokenEnv = withRepositoryReadFailure(env);
const failedSvg = await app.request("/v1/public/repos/acme/badged/badge.svg", {}, brokenEnv);
expect(failedSvg.status).toBe(503);
expect(failedSvg.headers.get("content-type")).toContain("image/svg+xml");
expect(failedSvg.headers.get("cache-control")).toBe("public, max-age=300");
expect(await failedSvg.text()).toContain("unavailable");

const failedJson = await app.request("/v1/public/repos/acme/badged/badge.json", {}, brokenEnv);
expect(failedJson.status).toBe(503);
expect(failedJson.headers.get("cache-control")).toBe("public, max-age=300");
await expect(failedJson.json()).resolves.toMatchObject({ schemaVersion: 1, label: "loopover", message: "unavailable", cacheSeconds: 300 });

// The healthy path is untouched by the new guard — still 200 with the long cache.
const stillOk = await app.request("/v1/public/repos/acme/badged/badge.svg", {}, env);
expect(stillOk.status).toBe(200);
expect(stillOk.headers.get("cache-control")).toContain("stale-while-revalidate");
});

it("serves public per-repo review-quality metrics only for installed, opted-in repos (#2568)", async () => {
Expand Down Expand Up @@ -7070,6 +7091,24 @@ function mcpHeaders(env: Env, sessionId?: string): Record<string, string> {
};
}

/** #8377: make the badge loader's first D1 read (getRepository) throw, so the route's own catch is what
* produces the response — the same targeted-`prepare` technique as {@link withProductUsageInsertFailure}. */
function withRepositoryReadFailure(env: Env): Env {
const db = env.DB as unknown as { prepare(sql: string): unknown; batch(statements: unknown[]): Promise<unknown> };
return {
...env,
DB: {
prepare(sql: string) {
if (sql.includes("repositories")) throw new Error("d1 blip reading repositories");
return db.prepare.call(db, sql);
},
batch(statements: unknown[]) {
return db.batch.call(db, statements);
},
} as unknown as D1Database,
};
}

function internalHeaders(env: Env): Record<string, string> {
return {
authorization: `Bearer ${env.INTERNAL_JOB_TOKEN}`,
Expand Down