fix(api): reject a non-positive-integer installation id on the seven id routes (#9740)

Seven routes parsed an installation id and validated it with Number.isFinite
alone, which accepts fractions and exponent notation -- Number("1.5") -> 1.5 and
0/negative all passed and were bound straight into the D1 lookup. A GitHub
installation id is always a positive integer, and the repo's own sibling routes
(the dead-letter-queue admin routes, the chat-qa :number param) already guard with
!Number.isInteger(id) || id <= 0.

Switch all seven -- GET/`/v1/installations/:id/{health,repair}`,
POST `.../repair/refresh`, and their four `/v1/app/installations/:id/*` siblings
(health, repair, repair/refresh, agent/bulk-settings) -- to that guard, returning
the existing { error: "invalid_installation_id" } body with 400 unchanged. The two
`/v1/internal/orb/*` routes that parse installationId from a JSON body are out of
scope and untouched.

Every one of the seven operations now declares 400 in the published OpenAPI
document (the four app routes in orb-and-control-route-specs.ts, the three legacy
routes' registerPath blocks in spec.ts); bulk-settings' existing 400 description is
widened to cover the id cause too. openapi.json is regenerated.

Adds a test asserting each of the seven routes rejects a fractional/zero/negative
id with 400 invalid_installation_id, while a valid positive integer still reaches
the handler.

Closes #9716

Author: shin-core

apps/loopover-ui/public/openapi.json

@@ -17365,6 +17365,9 @@
               }
             }
           },
+          "400": {
+            "description": "Malformed installation id"
+          },
           "404": {
             "description": "Installation health not found"
           }
@@ -17407,6 +17410,9 @@
               }
             }
           },
+          "400": {
+            "description": "Malformed installation id"
+          },
           "404": {
             "description": "Installation health not found"
           }
@@ -17449,6 +17455,9 @@
               }
             }
           },
+          "400": {
+            "description": "Malformed installation id"
+          },
           "404": {
             "description": "Installation not found"
           }
@@ -24011,6 +24020,9 @@
           "200": {
             "description": "Installation health"
           },
+          "400": {
+            "description": "Malformed installation id"
+          },
           "401": {
             "description": "Not signed in"
           },
@@ -24052,6 +24064,9 @@
           "200": {
             "description": "Repair plan"
           },
+          "400": {
+            "description": "Malformed installation id"
+          },
           "401": {
             "description": "Not signed in"
           },
@@ -24093,6 +24108,9 @@
           "200": {
             "description": "Repair plan recomputed"
           },
+          "400": {
+            "description": "Malformed installation id"
+          },
           "401": {
             "description": "Not signed in"
           },
@@ -24136,7 +24154,7 @@
             "description": "Settings applied"
           },
           "400": {
-            "description": "Malformed settings"
+            "description": "Malformed installation id or settings"
           },
           "401": {
             "description": "Not signed in"

src/api/routes.ts

@@ -2698,23 +2698,23 @@ export function createApp() {
 
   app.get("/v1/installations/:id/health", async (c) => {
     const installationId = Number(c.req.param("id"));
-    if (!Number.isFinite(installationId)) return c.json({ error: "invalid_installation_id" }, 400);
+    if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "invalid_installation_id" }, 400);
     const health = await getInstallationHealth(c.env, installationId);
     if (!health) return c.json({ error: "installation_health_not_found" }, 404);
     return c.json(enrichInstallationHealth(health));
   });
 
   app.get("/v1/installations/:id/repair", async (c) => {
     const installationId = Number(c.req.param("id"));
-    if (!Number.isFinite(installationId)) return c.json({ error: "invalid_installation_id" }, 400);
+    if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "invalid_installation_id" }, 400);
     const health = await getInstallationHealth(c.env, installationId);
     if (!health) return c.json({ error: "installation_health_not_found" }, 404);
     return c.json(await buildInstallationRepairDiagnostics(c.env, health));
   });
 
   app.post("/v1/installations/:id/repair/refresh", async (c) => {
     const installationId = Number(c.req.param("id"));
-    if (!Number.isFinite(installationId)) return c.json({ error: "invalid_installation_id" }, 400);
+    if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "invalid_installation_id" }, 400);
     const refreshed = await refreshInstallationHealthForInstallation(c.env, installationId);
     if (!refreshed) return c.json({ error: "installation_not_found" }, 404);
     const health = await getInstallationHealth(c.env, installationId);
@@ -2744,7 +2744,7 @@ export function createApp() {
     const resolved = await resolveAppInstallationScope(c);
     if (resolved instanceof Response) return resolved;
     const installationId = Number(c.req.param("id"));
-    if (!Number.isFinite(installationId)) return c.json({ error: "invalid_installation_id" }, 400);
+    if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "invalid_installation_id" }, 400);
     const health = await getInstallationHealth(c.env, installationId);
     if (!health) return c.json({ error: "installation_health_not_found" }, 404);
     if (!installationRecordInScope(resolved.scope, health)) return c.json({ error: "forbidden_installation" }, 403);
@@ -2755,7 +2755,7 @@ export function createApp() {
     const resolved = await resolveAppInstallationScope(c);
     if (resolved instanceof Response) return resolved;
     const installationId = Number(c.req.param("id"));
-    if (!Number.isFinite(installationId)) return c.json({ error: "invalid_installation_id" }, 400);
+    if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "invalid_installation_id" }, 400);
     const health = await getInstallationHealth(c.env, installationId);
     if (!health) return c.json({ error: "installation_health_not_found" }, 404);
     if (!installationRecordInScope(resolved.scope, health)) return c.json({ error: "forbidden_installation" }, 403);
@@ -2766,7 +2766,7 @@ export function createApp() {
     const resolved = await resolveAppInstallationScope(c);
     if (resolved instanceof Response) return resolved;
     const installationId = Number(c.req.param("id"));
-    if (!Number.isFinite(installationId)) return c.json({ error: "invalid_installation_id" }, 400);
+    if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "invalid_installation_id" }, 400);
     // Ownership is enforced BEFORE the refresh side effect so a tenant can never trigger repair on an
     // installation they don't own; the existing health record supplies the account the scope is checked against.
     const existing = await getInstallationHealth(c.env, installationId);
@@ -2788,7 +2788,7 @@ export function createApp() {
     const resolved = await resolveAppInstallationScope(c);
     if (resolved instanceof Response) return resolved;
     const installationId = Number(c.req.param("id"));
-    if (!Number.isFinite(installationId)) return c.json({ error: "invalid_installation_id" }, 400);
+    if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "invalid_installation_id" }, 400);
     const installation = await getInstallation(c.env, installationId);
     if (!installation) return c.json({ error: "installation_not_found" }, 404);
     if (!installationRecordInScope(resolved.scope, { installationId: installation.id, accountLogin: installation.accountLogin })) {

src/openapi/orb-and-control-route-specs.ts

@@ -174,7 +174,7 @@ const APP_ROUTES: SpecEntry[] = [
     tags: ["Control panel"],
     summary: "Return one installation's health summary",
     auth: "session",
-    responses: { 200: { description: "Installation health" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
+    responses: { 200: { description: "Installation health" }, 400: { description: "Malformed installation id" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
   },
   {
     method: "get",
@@ -183,7 +183,7 @@ const APP_ROUTES: SpecEntry[] = [
     tags: ["Control panel"],
     summary: "Return the repair plan for an unhealthy installation",
     auth: "session",
-    responses: { 200: { description: "Repair plan" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
+    responses: { 200: { description: "Repair plan" }, 400: { description: "Malformed installation id" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
   },
   {
     method: "post",
@@ -192,7 +192,7 @@ const APP_ROUTES: SpecEntry[] = [
     tags: ["Control panel"],
     summary: "Recompute an installation's repair plan",
     auth: "session",
-    responses: { 200: { description: "Repair plan recomputed" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
+    responses: { 200: { description: "Repair plan recomputed" }, 400: { description: "Malformed installation id" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
   },
   {
     method: "put",
@@ -201,7 +201,7 @@ const APP_ROUTES: SpecEntry[] = [
     tags: ["Control panel", "Agent automation"],
     summary: "Apply agent settings across every repo in an installation",
     auth: "session",
-    responses: { 200: { description: "Settings applied" }, 400: { description: "Malformed settings" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
+    responses: { 200: { description: "Settings applied" }, 400: { description: "Malformed installation id or settings" }, 404: { description: "No such installation" }, ...SESSION_AUTH_RESPONSES },
   },
   {
     method: "get",

src/openapi/spec.ts

@@ -540,6 +540,7 @@ export function buildOpenApiSpec() {
     request: { params: z.object({ id: z.string() }) },
     responses: {
       200: { description: "GitHub App installation health", content: { "application/json": { schema: InstallationHealthSchema } } },
+      400: { description: "Malformed installation id" },
       404: { description: "Installation health not found" },
     },
   });
@@ -552,6 +553,7 @@ export function buildOpenApiSpec() {
     request: { params: z.object({ id: z.string() }) },
     responses: {
       200: { description: "GitHub App installation repair diagnostics", content: { "application/json": { schema: InstallationRepairSchema } } },
+      400: { description: "Malformed installation id" },
       404: { description: "Installation health not found" },
     },
   });
@@ -564,6 +566,7 @@ export function buildOpenApiSpec() {
     request: { params: z.object({ id: z.string() }) },
     responses: {
       200: { description: "Refreshed GitHub App installation repair diagnostics", content: { "application/json": { schema: InstallationRepairSchema } } },
+      400: { description: "Malformed installation id" },
       404: { description: "Installation not found" },
     },
   });

test/integration/app-installations-selfservice.test.ts

@@ -170,6 +170,40 @@ describe("tenant self-service installation health/repair (#7661)", () => {
     await expect(missing.json()).resolves.toMatchObject({ error: "installation_health_not_found" });
   });
 
+  it("#9716: rejects a fractional/exponent/zero/negative installation id with 400 on every id route (Number.isFinite let those through)", async () => {
+    const app = createApp();
+    const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" });
+    await seedFleet(env);
+
+    // A GitHub installation id is always a positive integer. Number.isFinite accepted "1.5"->1.5 (fractional) and
+    // "0"/"-1" (non-positive), binding them straight into the D1 lookup; each must now be a 400. ("1e3"->1000 is a
+    // genuine integer and is deliberately NOT rejected -- it is exponent notation for a valid id, asserted below.)
+    const badIds = ["1.5", "0", "-1"];
+    const routes: Array<{ path: (id: string) => string; method: "GET" | "POST" | "PUT"; body?: string }> = [
+      { path: (id) => `/v1/installations/${id}/health`, method: "GET" },
+      { path: (id) => `/v1/installations/${id}/repair`, method: "GET" },
+      { path: (id) => `/v1/installations/${id}/repair/refresh`, method: "POST" },
+      { path: (id) => `/v1/app/installations/${id}/health`, method: "GET" },
+      { path: (id) => `/v1/app/installations/${id}/repair`, method: "GET" },
+      { path: (id) => `/v1/app/installations/${id}/repair/refresh`, method: "POST" },
+      { path: (id) => `/v1/app/installations/${id}/agent/bulk-settings`, method: "PUT", body: JSON.stringify({ autonomy: {} }) },
+    ];
+    for (const route of routes) {
+      for (const badId of badIds) {
+        const res = await app.request(
+          route.path(badId),
+          { method: route.method, headers: apiHeaders(env), ...(route.body !== undefined ? { body: route.body } : {}) },
+          env,
+        );
+        expect(res.status, `${route.method} ${route.path(badId)}`).toBe(400);
+        await expect(res.json()).resolves.toMatchObject({ error: "invalid_installation_id" });
+      }
+    }
+
+    // A valid positive integer id still passes the guard and reaches the handler (200 for the owned installation).
+    expect((await app.request("/v1/app/installations/600/health", { headers: apiHeaders(env) }, env)).status).toBe(200);
+  });
+
   it("scopes per-installation repair diagnostics and their error branches", async () => {
     const app = createApp();
     const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" });