Skip to content

api(installations): the installation routes accept a non-integer id and never publish their 400 #9716

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

Seven routes parse an installation id out of the path and validate it with Number.isFinite alone: GET /v1/installations/:id/health (src/api/routes.ts:2744-2745), GET /v1/installations/:id/repair (:2752-2753), POST /v1/installations/:id/repair/refresh (:2760-2761), GET /v1/app/installations/:id/health (:2790-2791), GET /v1/app/installations/:id/repair (:2801-2802), POST /v1/app/installations/:id/repair/refresh (:2812-2813), PUT /v1/app/installations/:id/agent/bulk-settings (:2834-2835).

Number.isFinite accepts fractions and exponent notation, so Number("1.5") -> 1.5 and Number("1e3") -> 1000 both pass and are bound straight into the D1 lookups. The repo's own sibling pattern for a numeric path id is stricter: the dead-letter-queue admin routes use if (!Number.isInteger(id) || id <= 0) return c.json({ error: "invalid_job_id" }, 400) (:2031, :2057), and the chat-qa route uses the same shape for :number (:3573-3574). A GitHub installation id is always a positive integer.

Separately, the 400 invalid_installation_id these routes return is declared for none of them. The four /v1/app/installations/* operations are specced in src/openapi/orb-and-control-route-specs.ts:170-205 (responses 200/401/403/404, plus 400 for bulk-settings' body only), and the three /v1/installations/* operations are legacy registerPath blocks in src/openapi/spec.ts declaring 200/404. A client sending a malformed id gets an undocumented status.

Requirements

  • All seven routes must reject an id that is not a positive integer, using !Number.isInteger(installationId) || installationId <= 0, returning the existing { error: "invalid_installation_id" } body with status 400. The error body and status must not change.
  • Every one of the seven operations must declare 400 in the published document. For PUT /v1/app/installations/{id}/agent/bulk-settings, which already declares a 400 for its body, the description must be widened to cover both causes.
  • No change to the ordering of the validation relative to the scope/ownership checks — the id check stays exactly where it is in each handler.

⚠️ Required pattern: copy the guard shape already used at src/api/routes.ts:2031 and :2057 (if (!Number.isInteger(id) || id <= 0) return c.json({ error: ... }, 400);). It does NOT satisfy this issue to add a zod schema for the path param; to introduce a shared helper in a new module; to change the error key from invalid_installation_id; or to fix the three /v1/installations/* routes without the four /v1/app/installations/* siblings (or vice versa).

Deliverables

  • All seven handlers listed in Context use !Number.isInteger(installationId) || installationId <= 0.
  • The three /v1/installations/{id}/* operations declare 400 in the regenerated apps/loopover-ui/public/openapi.json.
  • The four /v1/app/installations/{id}/* operations declare 400 there too.
  • A new test asserts GET /v1/installations/1.5/health answers 400 invalid_installation_id (it does not today).
  • A new test asserts GET /v1/app/installations/0/health and GET /v1/app/installations/-1/health both answer 400.
  • A regression test asserts a valid positive integer id still reaches the handler and returns its existing status for a known and an unknown installation (200 and 404).
  • apps/loopover-ui/public/openapi.json regenerated and committed.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example tightening the guards without declaring the 400s, or declaring the 400s without the fractional-id test — does not resolve this issue.

Test Coverage Requirements

src/api/** and src/openapi/** are inside Codecov's src/** include; the 99% branch-counted patch gate applies. Both arms of the tightened guard must be covered for at least one route per family (/v1/installations/* and /v1/app/installations/*), including the <= 0 arm and the non-integer arm separately, since they are distinct branches of the ||. The 1.5 case is the named regression test for this fix.

Expected Outcome

An installation id that is not a positive integer is rejected with the same 400 every other numeric path param in this app already returns, and that 400 is part of the published contract for all seven operations.

Links & Resources

src/api/routes.ts:2743-2845, :2028-2060, :3573-3574; src/openapi/orb-and-control-route-specs.ts:170-205; src/openapi/spec.ts; test/unit/routes-installation-bulk-agent-settings.test.ts.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions