Gap-hunt audit (PR #31/#32/#33 surface): 18 fixes + catalogued-entry cleanup

apps/api/src/agent-release-service.ts

@@ -52,6 +52,14 @@ interface AgentReleaseServiceOptions {
 const DEFAULT_TTL_MS = 30 * 60 * 1000;
 const DEFAULT_ERROR_TTL_MS = 5 * 60 * 1000;
 const DEFAULT_TIMEOUT_MS = 10_000;
+// The newest agent release is normally on page 1 (GitHub lists newest-first), but
+// a burst of docs/controller tags could push it back. Follow `Link: rel=next` a
+// bounded number of pages so we still find it without unbounded paging (R N-3A).
+const MAX_RELEASE_PAGES = 5;
+// Never buffer an unbounded response body into memory. 100 releases/page of
+// GitHub release JSON is well under this; a body larger than this from a
+// misconfigured/hostile API URL is rejected rather than read (R N-3B).
+const MAX_RELEASE_BODY_BYTES = 8 * 1024 * 1024;
 
 export function createAgentReleaseService(
   options: AgentReleaseServiceOptions = {},
@@ -72,22 +80,42 @@ export function createAgentReleaseService(
 
   async function doFetch(): Promise<void> {
     try {
-      const response = await fetchImpl(`${apiUrl}/repos/${repo}/releases?per_page=100`, {
-        headers: {
-          Accept: "application/vnd.github+json",
-          "User-Agent": "rakkr-controller",
-          "X-GitHub-Api-Version": "2022-11-28",
-          ...(token ? { Authorization: `Bearer ${token}` } : {}),
-        },
-        signal: AbortSignal.timeout(timeoutMs),
-      });
+      const firstUrl = `${apiUrl}/repos/${repo}/releases?per_page=100`;
+      const origin = new URL(firstUrl).origin;
+      const entries: unknown[] = [];
+      let url: string | null = firstUrl;
+
+      for (let page = 0; page < MAX_RELEASE_PAGES && url; page += 1) {
+        const response = await fetchImpl(url, {
+          headers: {
+            Accept: "application/vnd.github+json",
+            "User-Agent": "rakkr-controller",
+            "X-GitHub-Api-Version": "2022-11-28",
+            ...(token ? { Authorization: `Bearer ${token}` } : {}),
+          },
+          signal: AbortSignal.timeout(timeoutMs),
+        });
+
+        if (!response.ok) {
+          throw new Error(`github_releases_${response.status}`);
+        }
+
+        const parsed = JSON.parse(
+          await readBoundedText(response, MAX_RELEASE_BODY_BYTES),
+        ) as unknown;
+
+        if (Array.isArray(parsed)) {
+          entries.push(...parsed);
+        }
 
-      if (!response.ok) {
-        throw new Error(`github_releases_${response.status}`);
+        // Only follow a next link that stays on the same origin — don't chase a
+        // header pointing at an unrelated host.
+        const next = parseNextLink(response.headers.get("link"));
+
+        url = next && new URL(next).origin === origin ? next : null;
       }
 
-      const body = (await response.json()) as unknown;
-      const resolved = resolveLatestAgentRelease(body);
+      const resolved = resolveLatestAgentRelease(entries);
 
       if (resolved) {
         release = resolved;
@@ -160,6 +188,75 @@ export function resolveLatestAgentRelease(body: unknown): AgentRelease | null {
   return latest;
 }
 
+// Extracts the `rel="next"` URL from a GitHub `Link` header, or null when there
+// is no next page. GitHub emits `<url>; rel="next", <url>; rel="last"`.
+export function parseNextLink(linkHeader: string | null): string | null {
+  if (!linkHeader) {
+    return null;
+  }
+
+  for (const part of linkHeader.split(",")) {
+    const match = /<([^>]+)>\s*;\s*rel="?next"?/.exec(part);
+
+    if (match) {
+      return match[1];
+    }
+  }
+
+  return null;
+}
+
+// Reads a response body as text but refuses to buffer more than `maxBytes`,
+// checking the declared Content-Length first and then enforcing the cap while
+// streaming (chunked responses omit Content-Length).
+async function readBoundedText(response: Response, maxBytes: number): Promise<string> {
+  const declared = Number(response.headers.get("content-length"));
+
+  if (Number.isFinite(declared) && declared > maxBytes) {
+    throw new Error("github_releases_body_too_large");
+  }
+
+  const body = response.body;
+
+  if (!body) {
+    const text = await response.text();
+
+    if (byteLength(text) > maxBytes) {
+      throw new Error("github_releases_body_too_large");
+    }
+
+    return text;
+  }
+
+  const reader = body.getReader();
+  const decoder = new TextDecoder();
+  let received = 0;
+  let text = "";
+
+  for (;;) {
+    const { done, value } = await reader.read();
+
+    if (done) {
+      break;
+    }
+
+    received += value.byteLength;
+
+    if (received > maxBytes) {
+      await reader.cancel();
+      throw new Error("github_releases_body_too_large");
+    }
+
+    text += decoder.decode(value, { stream: true });
+  }
+
+  return text + decoder.decode();
+}
+
+function byteLength(value: string): number {
+  return new TextEncoder().encode(value).length;
+}
+
 let defaultService: AgentReleaseService | undefined;
 
 export function agentReleaseService(): AgentReleaseService {

apps/api/src/agent-route-helpers.ts

@@ -43,7 +43,18 @@ export const nodeHeartbeatSchema = z
   .object({
     agentVersion: z.string().trim().min(1).max(80),
     hostname: z.string().trim().min(1).max(255),
-    ipAddresses: z.array(z.string().trim().min(1).max(120)).max(16).default([]),
+    // A heartbeat is liveness-critical, so an over-cap ipAddresses list must not
+    // fail the whole heartbeat closed and strand the node as "offline". A
+    // multi-homed host's `hostname -I` can exceed 16 (IPv6 SLAAC/privacy addresses
+    // + Docker/libvirt/VLAN bridges); truncate to the documented cap and accept
+    // (the kept 16 are the primary addresses) rather than 400 every heartbeat
+    // forever and desync the node (audit R7-IPCAP).
+    ipAddresses: z
+      .preprocess(
+        (value) => (Array.isArray(value) ? value.slice(0, 16) : value),
+        z.array(z.string().trim().min(1).max(120)).max(16),
+      )
+      .default([]),
     runtime: nodeRuntimeSchema.optional(),
     status: nodeStatusSchema.default("online"),
   })

apps/api/src/agent-routes.ts

@@ -480,6 +480,18 @@ export function registerAgentRoutes({
 
   app.get("/api/v1/recording-jobs/:jobId", async (c, next) => {
     const jobId = c.req.param("jobId");
+
+    // `/api/v1/recording-jobs/export` is a static operator route that collides
+    // with this `:jobId` param route under Hono's TrieRouter (the app falls back
+    // to TrieRouter because of the nodes static+param collision — see audit G1).
+    // This node-auth handler is registered first, so without this guard it would
+    // answer `/export` with a node-credential 401 instead of the operator export
+    // handler. A real job id is never the literal "export", so defer it downstream.
+    if (jobId === "export") {
+      await next();
+      return c.res;
+    }
+
     const token = bearerToken(c.req.header("authorization"));
 
     if (!token)

apps/api/src/metrics.ts

@@ -1,11 +1,12 @@
-import type {
-  AuditEvent,
-  HealthEvent,
-  MeterFrame,
-  RecorderNode,
-  RecordingJob,
-  RecordingSummary,
-  UploadQueueItem,
+import {
+  isNodeReachable,
+  type AuditEvent,
+  type HealthEvent,
+  type MeterFrame,
+  type RecorderNode,
+  type RecordingJob,
+  type RecordingSummary,
+  type UploadQueueItem,
 } from "@rakkr/shared";
 
 import { nodeOfflineEventType, scheduledLowSignalEventType } from "./watchdog-runner.js";
@@ -45,7 +46,7 @@ export function renderPrometheusMetrics(input: PrometheusMetricsInput) {
   pushHelp(lines, "rakkr_node_online", "Whether a recorder node is reachable.");
   pushType(lines, "rakkr_node_online", "gauge");
   for (const node of input.nodes) {
-    pushMetric(lines, "rakkr_node_online", nodeLabels(node), node.status === "offline" ? 0 : 1);
+    pushMetric(lines, "rakkr_node_online", nodeLabels(node), isNodeReachable(node.status) ? 1 : 0);
   }
 
   pushHelp(lines, "rakkr_recording_active", "Active recording jobs by recorder node.");

apps/api/src/node-action-routes.ts

@@ -26,7 +26,10 @@ interface NodeActionState {
 }
 
 const monitorChunkMaxAgeMs = 5000;
-const unavailableNodeStatuses = new Set<RecorderNode["status"]>(["offline"]);
+// A provisioning node has never sent a heartbeat, so listen/meters/start are as
+// unavailable as an offline node — but the reason differs (it is not "offline",
+// it has never been online), so callers get an accurate label (audit H1-3).
+const unavailableNodeStatuses = new Set<RecorderNode["status"]>(["offline", "provisioning"]);
 
 export function registerNodeActionRoutes({
   app,
@@ -103,6 +106,7 @@ function nodeActions(
 ) {
   const basePath = `/api/v1/nodes/${node.id}`;
   const nodeAvailable = !unavailableNodeStatuses.has(node.status);
+  const unavailableReason = node.status === "provisioning" ? "node_provisioning" : "node_offline";
 
   return {
     detail: actionState({
@@ -132,15 +136,15 @@ function nodeActions(
       permission: "listen:monitor",
       permissions,
       ready: nodeAvailable && readiness.listen,
-      reason: nodeAvailable ? "monitor_source_unavailable" : "node_offline",
+      reason: nodeAvailable ? "monitor_source_unavailable" : unavailableReason,
     }),
     meters: actionState({
       href: `${basePath}/meters`,
       method: "GET",
       permission: "node:read",
       permissions,
       ready: nodeAvailable && readiness.meters,
-      reason: nodeAvailable ? "meter_frame_not_found" : "node_offline",
+      reason: nodeAvailable ? "meter_frame_not_found" : unavailableReason,
     }),
     rotateCredential: actionState({
       href: `${basePath}/credentials/rotate`,
@@ -156,7 +160,7 @@ function nodeActions(
       permission: "recording:create",
       permissions,
       ready: nodeAvailable,
-      reason: "node_offline",
+      reason: unavailableReason,
     }),
   };
 }

apps/api/src/node-routes.ts

@@ -11,6 +11,7 @@ import {
 } from "@rakkr/shared";
 
 import { registerAgentReleaseRoutes } from "./agent-release-routes.js";
+import type { AgentReleaseService } from "./agent-release-service.js";
 import type { AuthResult } from "./auth-service.js";
 import { buildMeterFrame, demoMetersEnabled } from "./demo-data.js";
 import type {
@@ -36,6 +37,7 @@ import type { NodeStore } from "./node-store.js";
 import { NodeStoreError } from "./node-store.js";
 
 interface NodeRouteDependencies {
+  agentReleaseService?: AgentReleaseService;
   app: Hono<AppBindings>;
   bootstrapStore: NodeBootstrapStore;
   currentAuth: (c: Context<AppBindings>) => AuthResult;
@@ -151,6 +153,7 @@ const nodeInterfaceUpdateSchema = z
   .refine(hasNodeUpdate, "At least one interface field is required");
 
 export function registerNodeRoutes({
+  agentReleaseService: releaseService,
   app,
   bootstrapStore,
   canServeWholeNodeMonitor = async () => true,
@@ -167,6 +170,18 @@ export function registerNodeRoutes({
   scopedNodes,
   sshCredentialStore,
 }: NodeRouteDependencies) {
+  // Register the static `/api/v1/nodes/agent-release` route BEFORE any
+  // `/api/v1/nodes/:nodeId` route. The node route set mixes a static child
+  // (`/export`) with a param child (`:nodeId`) at the same trie position, which
+  // Hono's RegExpRouter cannot represent, so the whole app falls back to the
+  // registration-order-sensitive TrieRouter. A static route registered AFTER
+  // `:nodeId` loses the match and gets swallowed by the detail handler (→ 404).
+  // Keeping this first mirrors how `/export` avoids the collision.
+  registerAgentReleaseRoutes({
+    agentReleaseService: releaseService,
+    app,
+    requirePermission,
+  });
   registerNodeInventoryRoutes({
     app,
     currentAuth,
@@ -193,10 +208,6 @@ export function registerNodeRoutes({
     requirePermission,
     scopedNodes,
   });
-  registerAgentReleaseRoutes({
-    app,
-    requirePermission,
-  });
   registerNodeSshCredentialRoutes({
     app,
     currentAuth,

apps/api/src/node-store-updates.ts

@@ -1,4 +1,4 @@
-import type { AudioInterface, RecorderNode } from "@rakkr/shared";
+import type { AudioInterface, NodeStatus, RecorderNode } from "@rakkr/shared";
 
 import { nonEmptyAudioDefaults } from "./node-metadata.js";
 import type {
@@ -11,6 +11,16 @@ import type {
 // paths. Extracted from node-store.ts to keep it under the LOC budget; the type
 // imports above are erased, so the store <-> updates edge is not a runtime cycle.
 
+// A heartbeat proves the node is in contact right now, so it must never leave
+// the node looking never-contacted (`provisioning`) or stale (`offline`) — the
+// controller owns the lifecycle state machine, so a first heartbeat promotes a
+// provisioning node to live, and a node (or a stale/rolled-back agent) cannot
+// self-report itself back out of offline detection (audit N4). Other live
+// statuses the agent may report (recording/degraded/alerting) pass through.
+export function heartbeatStatus(status: NodeStatus): NodeStatus {
+  return status === "provisioning" || status === "offline" ? "online" : status;
+}
+
 export function updatedNodeHeartbeat(node: RecorderNode, input: NodeHeartbeatInput): RecorderNode {
   return {
     ...node,
@@ -19,7 +29,7 @@ export function updatedNodeHeartbeat(node: RecorderNode, input: NodeHeartbeatInp
     ipAddresses: input.ipAddresses,
     lastSeenAt: new Date().toISOString(),
     runtime: input.runtime ?? node.runtime,
-    status: input.status,
+    status: heartbeatStatus(input.status),
   };
 }
 

apps/api/src/node-store.ts

@@ -39,7 +39,12 @@ import {
   recorderInterfaceToRow,
   recorderNodeToRow,
 } from "./node-store-mappers.js";
-import { updatedNode, updatedNodeHeartbeat, updatedNodeInterface } from "./node-store-updates.js";
+import {
+  heartbeatStatus,
+  updatedNode,
+  updatedNodeHeartbeat,
+  updatedNodeInterface,
+} from "./node-store-updates.js";
 
 export interface NodeEnrollmentInput {
   agentVersion: string;
@@ -448,7 +453,7 @@ class PostgresNodeStore implements NodeStore {
           lastSeenAt: new Date(),
           metadata: nodeMetadata(row.metadata, nodeRuntimeFromInput(input.runtime, row.metadata)),
           network: { ipAddresses: input.ipAddresses },
-          status: input.status,
+          status: heartbeatStatus(input.status),
           updatedAt: new Date(),
         })
         .where(eq(nodeRows.id, nodeId));

apps/api/src/schedule-route-helpers.ts

@@ -62,7 +62,9 @@ export function buildSchedule(input: ScheduleInput): ScheduleSummary {
     tags: uniqueTags(input.tags),
     timezone: input.timezone,
     titleTemplate: input.titleTemplate,
-    uploadPolicyIds: input.uploadPolicyIds,
+    // Dedup server-side (the client also dedups) so the server is authoritative:
+    // each id fans a recording out to its own upload queue item (audit R4-1).
+    uploadPolicyIds: [...new Set(input.uploadPolicyIds)],
     watchdogPolicyId: input.watchdogPolicyId,
   };
 }
@@ -105,6 +107,10 @@ export function sanitizeScheduleUpdate(
     updates.tags = uniqueTags(input.tags);
   }
 
+  if (input.uploadPolicyIds) {
+    updates.uploadPolicyIds = [...new Set(input.uploadPolicyIds)];
+  }
+
   return updates;
 }