From 19329236e6a0a3bb734f083dc70d2dd94d17d338 Mon Sep 17 00:00:00 2001 From: ibolton336 Date: Wed, 5 Aug 2026 11:54:34 -0400 Subject: [PATCH 1/2] feat(hub-shim): label runs with their application and filter on it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #3. An application-scoped run carried its application only in spec.env as APP_ID, so "which runs belong to application 42?" had no server-side answer — a client had to fetch every run and scan spec.env, which is what the UI's per-application drawer does today. Fine at demo scale, wrong at inventory scale, and on the path of every per-application view. Stamp konveyor.io/application: "" at create time on both AgentRun and AgentWorkflowRun, and serve ?application= on both run lists as a label selector the apiserver evaluates. The label is additive to APP_ID, not a replacement: the env var is how the pod learns its application (the harness resolves from Hub at runtime), the label is how the API indexes runs. An env var is not indexable; a label is not visible inside the container. Three things worth calling out: - The numeric-id check moves to invalidApplicationIdReason() and now also guards the AgentWorkflowRun create path, which had none. That path gains a 400 for non-numeric Hub ids — previously the apiserver would have rejected the create outright once a label carried the value. - ?application= on a resource that cannot honour it is a 400, not a silent pass. An ignored filter returns every run, which a per-application view renders as "these all belong to 42" — a wrong answer that looks right. - BadRequestError now maps to 400 at the top-level catch rather than falling through to 500. Known gap: the controller builds each stage AgentRun's labels from scratch instead of inheriting the parent's, so workflow stage runs do not carry the label and ?application= on /api/agentruns finds single runs only. That needs an upstream controller change. Verified end-to-end against a real kube-apiserver (envtest binaries, no cluster): label stored, kubectl -l selects correctly, pre-label runs excluded, all four 400 paths. dev/application-filter-smoke.ts (npm run smoke:appfilter) is the reusable version. ADR 0009's route table and ADR 0010's label section record the contract; the matching amendment to upstream ADR 0006 is filed separately. Co-Authored-By: Claude Opus 5 --- .../0009-client-contract-and-transports.md | 11 +- docs/adr/0010-platform-resolved-params.md | 35 +++ packages/agentic-client/src/contract/index.ts | 26 ++ .../hub-shim/dev/application-filter-smoke.ts | 238 ++++++++++++++++++ packages/hub-shim/package.json | 3 +- packages/hub-shim/src/server.ts | 81 +++++- 6 files changed, 378 insertions(+), 16 deletions(-) create mode 100644 packages/hub-shim/dev/application-filter-smoke.ts diff --git a/docs/adr/0009-client-contract-and-transports.md b/docs/adr/0009-client-contract-and-transports.md index 4eff633..3f6ddf4 100644 --- a/docs/adr/0009-client-contract-and-transports.md +++ b/docs/adr/0009-client-contract-and-transports.md @@ -100,13 +100,18 @@ passthrough proxy is expected to expose: | GET | `/api/applications` | 200 `Application[]` — the platform's application inventory (mocked in the shim; Hub serves its real records). Source of truth for resolved params/credentials, see ADR 0010. | | GET | `/api/agents` | 200 `AgentResource[]` (full CRs, metadata+spec), **filtered to `konveyor.io/managed=true`** | | GET | `/api/agents/:name` | 200 `AgentResource` \| 404 (never label-filtered) | -| GET | `/api/llmproviders[/:name]` | 200 `LLMProvider[]` \| `LLMProvider` \| 404 | +| GET | `/api/gateways[/:name]` | 200 `Gateway[]` \| `Gateway` \| 404 (was `/api/llmproviders` before the #100 rename) | | GET | `/api/skillcards[/:name]` | 200 `SkillCard[]` \| `SkillCard` \| 404 | | GET | `/api/skillcollections[/:name]` | 200 `SkillCollection[]` \| `SkillCollection` \| 404 | -| GET | `/api/agentruns` | 200 `AgentRun[]` (full CRs) | -| POST | `/api/agentruns` (body `{agentRef, params?: Record, instructions?, applicationRef?}`) | 201 `AgentRun` (generateName `ui-`, params mapped to `[{name,value}]`). When `applicationRef` is set, the platform resolves the Agent's declared param/credential sources from that application (ADR 0010): resolved params merge under caller-supplied ones, credentials become `spec.envFrom`. 400 on unknown `applicationRef`, or a required param with a recognized source the application cannot supply. | +| GET | `/api/agentruns[?application=]` | 200 `AgentRun[]` (full CRs). `application` filters by the `konveyor.io/application` label (ADR 0010) — a `client.List()` label selector, never a client-side scan. Runs predating the label are not selected. 400 on a non-numeric id; 400 on any resource that cannot honour the filter, never a silent unfiltered list. | +| POST | `/api/agentruns` (body `{agentRef, params?: Record, instructions?, applicationRef?, targetBranch?, gateway?}`) | 201 `AgentRun` (generateName `ui-`, params mapped to `[{name,value}]`). When `applicationRef` is set, the platform resolves the Agent's declared param/credential sources from that application (ADR 0010): resolved params merge under caller-supplied ones, credentials become `spec.envFrom`, and the run is stamped `konveyor.io/application: ""`. 400 on unknown `applicationRef`, or a required param with a recognized source the application cannot supply. | | GET | `/api/agentruns/:name` | 200 `AgentRun` \| 404 | | DELETE | `/api/agentruns/:name` | 204 | +| GET | `/api/agentworkflows[/:name]` | 200 `AgentWorkflow[]` \| `AgentWorkflow` \| 404 (list **filtered to `konveyor.io/managed=true`**) | +| GET | `/api/agentworkflowruns[?application=]` | 200 `AgentWorkflowRun[]`. Same `application` semantics as `/api/agentruns`; composes with the managed filter as one selector. **Stage runs do not carry the parent's application label** — the controller builds their labels from scratch — so filtering `agentruns` finds single runs only. | +| GET | `/api/agentworkflowruns/:name` | 200 `AgentWorkflowRun` \| 404 | +| POST | `/api/agentworkflowruns` (body `{workflowRef, params?, applicationRef?, targetBranch?, gateway?}`) | 201 `AgentWorkflowRun` (generateName `ui-`), labelled `konveyor.io/managed` plus `konveyor.io/application` when scoped. | +| DELETE | `/api/agentworkflowruns/:name` | 204 | | WS | `/api/agentruns/:name/acp` | Resolves the run's ACP endpoint (waitForAcpEndpoint semantics, 60s), opens a port-forward tunnel to the pod, dials `ws://127.0.0.1:/acp` upstream WITH `X-Secret-Key` (key read from the run's Secret), then pipes frames bidirectionally. Client close → close upstream + tunnel; upstream close/error → close client 1011 with reason. | The shim itself is unauthenticated (localhost dev tool) and serves diff --git a/docs/adr/0010-platform-resolved-params.md b/docs/adr/0010-platform-resolved-params.md index 812622d..b791a68 100644 --- a/docs/adr/0010-platform-resolved-params.md +++ b/docs/adr/0010-platform-resolved-params.md @@ -38,6 +38,41 @@ Agents that Konveyor UIs know how to drive carry (`GET /api/agents` in SHIM API v1 does); unlabeled Agents remain usable by other consumers and invisible to Konveyor UIs. +**Runs carry a second platform label: `konveyor.io/application: ""`**, +written at create time on both AgentRun and AgentWorkflowRun whenever the +caller supplies an application, so per-application run views are a label +selector instead of a scan. Same reasoning as the managed label — a +namespaced label the platform owns, no CRD schema involvement. + +This is **additive to `APP_ID` in `spec.env`, not a replacement**. The two +serve different consumers and neither can do the other's job: + +| | carrier | consumer | why it can't be the other | +|---|---|---|---| +| `APP_ID` | `spec.env` | the **pod** — the harness resolves the application from Hub at runtime | env vars are not indexable; answering "runs for app 42" means fetching every run and parsing `spec.env` | +| `konveyor.io/application` | `metadata.labels` | the **API** — `?application=` becomes a `client.List()` label selector | a label is not visible inside the container | + +Consequences, all verified in the shim prototype: + +- Hub application ids are numeric (the harness's `APP_ID` parser requires a + uint), which makes them valid label values with no sanitization. One + numeric check covers both concerns: a run the harness would reject at + startup, and a create the apiserver would reject outright. +- Runs created before the label are invisible to the selector. A filtered + list is "runs we can prove belong to 42", not "every run that ever + touched 42". Callers needing the old runs keep a `spec.env` fallback for + one release. +- **Workflow stage runs do not inherit it.** The controller builds each + stage AgentRun's labels from scratch, so filtering `agentruns` finds + single runs only. Propagating parent labels to stage runs is an upstream + controller change, tracked separately. + +Status upstream: konveyor/agentic-controller ADR 0006 currently specifies +the opposite — "the application ID goes directly on the CR as an env var" +and "other resource types are listed unfiltered". That ADR is *proposed*; +this section is the amendment being taken to it. Until it lands, the label +exists only in the shim. + ### (b) Param sources: generic field, namespaced values, no enum A param may declare a **source identifier** — a free-form, namespaced diff --git a/packages/agentic-client/src/contract/index.ts b/packages/agentic-client/src/contract/index.ts index dd43377..d662971 100644 --- a/packages/agentic-client/src/contract/index.ts +++ b/packages/agentic-client/src/contract/index.ts @@ -203,6 +203,18 @@ function decodeBase64Utf8(b64: string, keyName: string): string { */ export const MANAGED_LABEL = "konveyor.io/managed"; +/** + * Label stamping a run with the Hub application it works on, so "which runs + * belong to application 42?" is a server-side label selector rather than a + * fetch-everything-and-scan-spec.env walk. Written at create time on both + * AgentRun and AgentWorkflowRun; the value is the Hub application id. + * + * Runs created before this label existed do not carry it and are invisible + * to the selector — a filtered list is "runs we can prove belong to 42", + * not "every run that ever touched 42". + */ +export const APPLICATION_LABEL = "konveyor.io/application"; + /** * Agent annotation mapping param name -> source identifier, e.g. * {"repository": "konveyor.io/application-repository-url"}. A param with a @@ -442,6 +454,20 @@ export function invalidTargetBranchReason(branch: string): string | undefined { return undefined; } +/** + * Why this application id cannot be used for a run, or undefined when it + * can. Hub ids are numeric and the harness's APP_ID parser requires a uint, + * so a non-numeric id is a run that dies at startup — and, since the id is + * also stamped as the {@link APPLICATION_LABEL} value, a create the + * apiserver would reject outright. One check covers both. + */ +export function invalidApplicationIdReason(id: string): string | undefined { + if (!/^\d+$/.test(id)) { + return `application id "${id}" is not numeric — the harness's APP_ID parser requires a uint Hub id`; + } + return undefined; +} + export const RUN_ENV = { HUB_BASE_URL: "HUB_BASE_URL", HUB_TOKEN: "HUB_TOKEN", diff --git a/packages/hub-shim/dev/application-filter-smoke.ts b/packages/hub-shim/dev/application-filter-smoke.ts new file mode 100644 index 0000000..29f5947 --- /dev/null +++ b/packages/hub-shim/dev/application-filter-smoke.ts @@ -0,0 +1,238 @@ +/** + * E2E smoke for the per-application run filter against a LIVE shim + cluster. + * + * Browser-constraint like dev/browser-smoke.ts: only globalThis.fetch, only + * shim routes — every cluster assertion goes through the shim's own GET + * endpoints, exactly what a browser UI could verify. + * + * Asserts the konveyor.io/application contract (client#3): + * 1. an application-scoped AgentRun is stamped with the label at create + * 2. an application-scoped AgentWorkflowRun is too, without losing managed + * 3. a run created with no applicationRef carries no application label + * 4. ?application= returns that application's runs and nothing else + * 5. the filter is server-side — a foreign id returns an empty list while + * the unfiltered list still holds the runs + * 6. a filter the endpoint cannot honour is a 400, never a silent full list + * + * Needs a REAL Hub inventory: application-scoped creates are refused against + * the offline stub, so the smoke skips itself when the shim reports one. + * + * Run with the shim already up: npm run smoke:appfilter (SHIM_URL overrides) + * Deletes every run it creates. + */ + +const BASE = process.env.SHIM_URL ?? "http://127.0.0.1:7080"; +const APPLICATION_LABEL = "konveyor.io/application"; + +let failures = 0; +function pass(step: string, detail?: string): void { + console.log(`PASS ${step}${detail ? ` — ${detail}` : ""}`); +} +function fail(step: string, detail: string): never { + failures++; + console.error(`FAIL ${step} — ${detail}`); + throw new SmokeAbort(step); +} +class SmokeAbort extends Error { + constructor(step: string) { + super(`aborted at step: ${step}`); + } +} + +interface NamedCR { + metadata: { name: string; labels?: Record }; +} +interface ShimApplication { + id: string; + name: string; + repository?: { url?: string; branch?: string }; +} + +async function call( + method: string, + path: string, + body?: unknown, +): Promise<{ status: number; body: unknown }> { + const res = await fetch(`${BASE}${path}`, { + method, + headers: body === undefined ? undefined : { "content-type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + const text = await res.text(); + try { + return { status: res.status, body: JSON.parse(text) as unknown }; + } catch { + return { status: res.status, body: text }; + } +} + +async function get(path: string): Promise { + const res = await call("GET", path); + if (res.status !== 200) fail(`GET ${path}`, `HTTP ${res.status} ${JSON.stringify(res.body)}`); + return res.body as T; +} + +const names = (list: NamedCR[]) => list.map((r) => r.metadata.name).sort(); +const has = (list: NamedCR[], name: string) => list.some((r) => r.metadata.name === name); + +/** Runs created by this smoke, torn down in reverse order. */ +const created: Array<{ plural: string; name: string }> = []; + +async function createRun( + plural: string, + body: Record, + step: string, +): Promise { + const res = await call("POST", `/api/${plural}`, body); + if (res.status !== 201) fail(step, `HTTP ${res.status} ${JSON.stringify(res.body)}`); + const run = res.body as NamedCR; + created.push({ plural, name: run.metadata.name }); + return run; +} + +async function main(): Promise { + // -- inventory: the filter is only meaningful against real Hub ids + const inventoryRes = await fetch(`${BASE}/api/applications`); + if (!inventoryRes.ok) fail("inventory", `GET /api/applications -> HTTP ${inventoryRes.status}`); + if (inventoryRes.headers.get("X-Inventory-Source") !== "hub") { + console.log( + "application-filter-smoke: SKIPPED — shim is serving the offline stub inventory, " + + "which refuses application-scoped runs. Point HUB_URL at a reachable Hub.", + ); + return; + } + const applications = ((await inventoryRes.json()) as ShimApplication[]).filter((a) => + /^\d+$/.test(a.id), + ); + if (applications.length < 2) { + fail("inventory", `need two numeric-id applications to prove isolation, got ${applications.length}`); + } + const [appA, appB] = applications; + pass("inventory", `filtering between application ${appA.id} and ${appB.id}`); + + // A managed agent is the one a real per-application view drives. + const agents = await get("/api/agents"); + if (agents.length === 0) fail("agent fixture", "no managed agents — POST /api/defaults first"); + const agentRef = agents[0].metadata.name; + + try { + // -- 1. the single-run create stamps the label + const runA = await createRun( + "agentruns", + { agentRef, applicationRef: appA.id }, + "create AgentRun", + ); + if (runA.metadata.labels?.[APPLICATION_LABEL] !== appA.id) { + fail("AgentRun label", `expected ${appA.id}, got ${runA.metadata.labels?.[APPLICATION_LABEL]}`); + } + pass("AgentRun label", `${runA.metadata.name} -> ${APPLICATION_LABEL}=${appA.id}`); + + const runB = await createRun( + "agentruns", + { agentRef, applicationRef: appB.id }, + "create AgentRun (second application)", + ); + + // -- 2/3. the workflow-run create stamps it too, and only when scoped + const workflows = await get("/api/agentworkflows"); + let workflowRunA: NamedCR | undefined; + if (workflows.length === 0) { + pass("AgentWorkflowRun label", "skipped — no managed workflows on this cluster"); + } else { + const workflowRef = workflows[0].metadata.name; + workflowRunA = await createRun( + "agentworkflowruns", + { workflowRef, applicationRef: appA.id }, + "create AgentWorkflowRun", + ); + const labels = workflowRunA.metadata.labels ?? {}; + if (labels[APPLICATION_LABEL] !== appA.id) { + fail("AgentWorkflowRun label", `expected ${appA.id}, got ${labels[APPLICATION_LABEL]}`); + } + if (labels["konveyor.io/managed"] !== "true") { + fail("AgentWorkflowRun label", "application label displaced the managed label"); + } + pass("AgentWorkflowRun label", `${workflowRunA.metadata.name} keeps managed + application`); + + const unscoped = await createRun( + "agentworkflowruns", + { workflowRef }, + "create unscoped AgentWorkflowRun", + ); + if (unscoped.metadata.labels?.[APPLICATION_LABEL] !== undefined) { + fail("unscoped run", `expected no application label, got ${unscoped.metadata.labels?.[APPLICATION_LABEL]}`); + } + pass("unscoped run", "no applicationRef -> no application label"); + } + + // -- 4/5. the filter isolates, and the unfiltered list is unaffected + const filteredA = await get(`/api/agentruns?application=${appA.id}`); + if (!has(filteredA, runA.metadata.name) || has(filteredA, runB.metadata.name)) { + fail( + `?application=${appA.id}`, + `expected ${runA.metadata.name} without ${runB.metadata.name}, got ${names(filteredA).join(", ")}`, + ); + } + pass(`?application=${appA.id}`, `${filteredA.length} run(s), ${runB.metadata.name} excluded`); + + const filteredB = await get(`/api/agentruns?application=${appB.id}`); + if (!has(filteredB, runB.metadata.name) || has(filteredB, runA.metadata.name)) { + fail(`?application=${appB.id}`, `leaked across applications: ${names(filteredB).join(", ")}`); + } + pass(`?application=${appB.id}`, `${filteredB.length} run(s), ${runA.metadata.name} excluded`); + + // An id no run carries proves the apiserver is doing the selecting. + const foreign = await get("/api/agentruns?application=999999"); + if (foreign.length !== 0) { + fail("unmatched application", `expected [], got ${names(foreign).join(", ")}`); + } + pass("unmatched application", "empty list, not a silent full list"); + + const unfiltered = await get("/api/agentruns"); + if (!has(unfiltered, runA.metadata.name) || !has(unfiltered, runB.metadata.name)) { + fail("unfiltered list", "filtering removed runs from the unfiltered list"); + } + pass("unfiltered list", `${unfiltered.length} run(s) with both applications present`); + + if (workflowRunA) { + const wfrA = await get(`/api/agentworkflowruns?application=${appA.id}`); + if (!has(wfrA, workflowRunA.metadata.name)) { + fail(`agentworkflowruns?application=${appA.id}`, `missing ${workflowRunA.metadata.name}`); + } + const wfrB = await get(`/api/agentworkflowruns?application=${appB.id}`); + if (has(wfrB, workflowRunA.metadata.name)) { + fail(`agentworkflowruns?application=${appB.id}`, "leaked across applications"); + } + pass("agentworkflowruns filter", `isolated ${appA.id} from ${appB.id}`); + } + + // -- 6. an unhonourable filter is loud + const rejects: Array<[string, string]> = [ + ["non-run resource", "/api/agents?application=" + appA.id], + ["empty value", "/api/agentruns?application="], + ["non-numeric id", "/api/agentruns?application=not-a-number"], + ]; + for (const [label, path] of rejects) { + const res = await call("GET", path); + if (res.status !== 400) { + fail(`400 on ${label}`, `expected 400, got HTTP ${res.status} for ${path}`); + } + pass(`400 on ${label}`, String((res.body as { error?: string })?.error ?? res.status)); + } + } finally { + for (const { plural, name } of created.reverse()) { + const res = await call("DELETE", `/api/${plural}/${name}`); + if (res.status !== 204) console.error(`WARN leaked ${plural}/${name} (HTTP ${res.status})`); + } + if (created.length > 0) pass("cleanup", `deleted ${created.length} run(s)`); + } +} + +main() + .then(() => { + if (failures === 0) console.log("application-filter-smoke: all checks passed"); + }) + .catch((err) => { + if (!(err instanceof SmokeAbort)) console.error(`FAIL unexpected — ${err}`); + process.exitCode = 1; + }); diff --git a/packages/hub-shim/package.json b/packages/hub-shim/package.json index cf477c7..ed10bd3 100644 --- a/packages/hub-shim/package.json +++ b/packages/hub-shim/package.json @@ -8,7 +8,8 @@ "start": "tsx src/server.ts", "typecheck": "tsc --noEmit", "smoke": "tsx dev/browser-smoke.ts", - "smoke:defaults": "tsx dev/defaults-smoke.ts" + "smoke:defaults": "tsx dev/defaults-smoke.ts", + "smoke:appfilter": "tsx dev/application-filter-smoke.ts" }, "dependencies": { "@kubernetes/client-node": "^1.4.0", diff --git a/packages/hub-shim/src/server.ts b/packages/hub-shim/src/server.ts index 53cc54e..33d7cfb 100644 --- a/packages/hub-shim/src/server.ts +++ b/packages/hub-shim/src/server.ts @@ -29,6 +29,13 @@ * GET /api/agentruns/:name -> 200 AgentRun | 404 * DELETE /api/agentruns/:name -> 204 | 404 * WS /api/agentruns/:name/acp -> bidirectional pipe to the pod + * GET /api/agentworkflowruns[/:name]-> 200 AgentWorkflowRun[] | AgentWorkflowRun | 404 + * POST /api/agentworkflowruns -> 201 AgentWorkflowRun + * DELETE /api/agentworkflowruns/:name -> 204 | 404 + * + * Both run lists take `?application=`, answered by the + * konveyor.io/application label stamped at create time — runs predating the + * label are not selected. The filter is a 400 on any other resource. * * No auth on the shim itself — localhost dev tool only. CORS `*` on /api/*. */ @@ -54,6 +61,7 @@ import { type Gateway, } from "../../agentrun-client/src/types.js"; import { + APPLICATION_LABEL, CREDENTIAL_SOURCES_ANNOTATION, MANAGED_LABEL, PARAM_SOURCES_ANNOTATION, @@ -62,6 +70,7 @@ import { SOURCE_APPLICATION_REPOSITORY_BRANCH, SOURCE_APPLICATION_REPOSITORY_URL, defaultTargetBranch, + invalidApplicationIdReason, invalidTargetBranchReason, parseSourcesAnnotation, type AgentImage, @@ -150,6 +159,32 @@ const LIST_LABEL_SELECTORS: Record = { [PLURALS.AgentWorkflow]: `${MANAGED_LABEL}=true`, }; +/** Run kinds whose list endpoint answers `?application=`. */ +const APPLICATION_FILTERABLE: readonly string[] = [PLURALS.AgentRun, PLURALS.AgentWorkflowRun]; + +/** + * Translates `?application=42` into the label selector that answers it in + * the apiserver, or undefined when the caller did not ask to filter. + * + * A filter this endpoint cannot honour is a 400, never a silent pass: the + * unfiltered response is every run in the namespace, which a caller that + * asked for one application's runs would render as exactly that. + */ +function applicationSelector(req: http.IncomingMessage, plural: string): string | undefined { + const raw = new URL(req.url ?? "/", "http://localhost").searchParams.get("application"); + if (raw === null) return undefined; + if (!APPLICATION_FILTERABLE.includes(plural)) { + badRequest( + `?application= filters runs only — supported on ${APPLICATION_FILTERABLE.map((p) => `/api/${p}`).join(", ")}`, + ); + } + const id = raw.trim(); + if (!id) badRequest("?application= needs an application id (GET /api/applications lists the inventory)"); + const problem = invalidApplicationIdReason(id); + if (problem) badRequest(problem); + return `${APPLICATION_LABEL}=${id}`; +} + /** * Real Konveyor Hub REST base. In-cluster this is the Hub service DNS * (http://tackle2-hub..svc:8080); on a laptop, a port-forward or @@ -585,12 +620,8 @@ async function hubEnvForRun( `unknown applicationRef "${applicationRef}" — GET /api/applications lists the inventory`, ); } - if (!/^\d+$/.test(app.id)) { - badRequest( - `application id "${app.id}" is not numeric — the harness's APP_ID parser requires a ` + - `uint Hub id`, - ); - } + const idProblem = invalidApplicationIdReason(app.id); + if (idProblem) badRequest(idProblem); if (!app.repository?.url) { badRequest( `application "${app.id}" has no repository URL — the harness clones from the Hub ` + @@ -1112,6 +1143,11 @@ async function handleApi( if (inv.source === "stub" || !inv.endpoint) { return sendError(res, 400, "applicationRef needs a real Hub inventory (current source is the built-in stub) — the run's stages resolve the repo from the Hub"); } + // Same guard the single-run path applies via hubEnvForRun: a + // non-numeric id is a stage harness that dies on APP_ID, and an + // application label the apiserver would reject on create. + const idProblem = invalidApplicationIdReason(app.id); + if (idProblem) return sendError(res, 400, idProblem); env.push( { name: "HUB_BASE_URL", value: inv.endpoint }, { name: "APP_ID", value: app.id }, @@ -1138,7 +1174,14 @@ async function handleApi( metadata: { generateName: "ui-", namespace: NAMESPACE, - labels: { [MANAGED_LABEL]: "true" }, + labels: { + [MANAGED_LABEL]: "true", + // Queryable application link. NOTE: the controller builds each + // stage AgentRun's labels from scratch rather than inheriting + // the parent's, so stage runs do NOT carry this — filtering + // /api/agentruns finds single runs, not workflow stages. + ...(input.applicationRef ? { [APPLICATION_LABEL]: input.applicationRef } : {}), + }, }, spec, }, @@ -1172,7 +1215,12 @@ async function handleApi( const plural = apiMatch[1]; const kind = READ_ONLY[plural]!; if (!apiMatch[2]) { - return sendJson(res, 200, await listCustom(plural, kind, LIST_LABEL_SELECTORS[plural])); + // Requirements AND: a managed-catalog filter and an application filter + // compose into one selector the apiserver evaluates. + const selector = [LIST_LABEL_SELECTORS[plural], applicationSelector(req, plural)] + .filter(Boolean) + .join(","); + return sendJson(res, 200, await listCustom(plural, kind, selector || undefined)); } const name = decodeURIComponent(apiMatch[2]); try { @@ -1185,7 +1233,8 @@ async function handleApi( if (pathname === "/api/agentruns") { if (method === "GET") { - return sendJson(res, 200, await listCustom(PLURALS.AgentRun, "AgentRun")); + const selector = applicationSelector(req, PLURALS.AgentRun); + return sendJson(res, 200, await listCustom(PLURALS.AgentRun, "AgentRun", selector)); } if (method === "POST") { let input: CreateRunBody; @@ -1234,7 +1283,13 @@ async function handleApi( // LLM credential itself since #100). if (hubEnv.length > 0) spec.env = hubEnv; if (sources.envFrom.length > 0) spec.envFrom = sources.envFrom; - const run = await runClient.createAgentRun(spec, { generateName: "ui-" }); + // The application also rides a label so per-application views are a + // selector instead of a scan of every run's spec.env. applicationRef + // is the id hubEnvForRun just matched and validated as numeric. + const run = await runClient.createAgentRun(spec, { + generateName: "ui-", + labels: input.applicationRef ? { [APPLICATION_LABEL]: input.applicationRef } : undefined, + }); const via = input.applicationRef ? ` via application=${input.applicationRef}` : ""; log(`created AgentRun ${run.metadata.name} (agentRef=${input.agentRef}${via})`); return sendJson(res, 201, run); @@ -1297,8 +1352,10 @@ const server = http.createServer((req, res) => { } handleApi(req, res, pathname).catch((err: unknown) => { - const status = k8sStatusCode(err) ?? 500; - warn(`${req.method} ${pathname} failed: ${errorMessage(err)}`); + // Handlers that validate inline own their own 400s; a BadRequestError + // thrown past them is still a caller fault, not a 500. + const status = err instanceof BadRequestError ? 400 : (k8sStatusCode(err) ?? 500); + warn(`${req.method} ${pathname} ${status === 400 ? "rejected" : "failed"}: ${errorMessage(err)}`); if (!res.headersSent) sendError(res, status, errorMessage(err)); else res.end(); }); From e0addd5dd46a144308f807f19bb765b6aaf6b643 Mon Sep 17 00:00:00 2001 From: ibolton336 Date: Wed, 5 Aug 2026 12:10:34 -0400 Subject: [PATCH 2/2] fix: bound application ids to uint64, not just digits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on the upstream ADR amendment (konveyor/agentic-controller#105) caught a real gap: the guard's /^\d+$/ accepted any digit string, but the harness's hub.ParseAppID is strconv.ParseUint(s, 10, 64) — a 21-digit id passed the shim and then killed the run at startup, the exact doomed-run class the guard exists to pre-empt. Unbounded digits could also exceed the apiserver's 63-char label-value limit. invalidApplicationIdReason now rejects values above uint64 max (BigInt compare, exact at the boundary: ...551615 valid, ...551616 rejected). Smoke gains the overflow reject case; ADR 0010's claim is restated with the bound that makes it true. Co-Authored-By: Claude Opus 5 --- docs/adr/0010-platform-resolved-params.md | 10 ++++++---- packages/agentic-client/src/contract/index.ts | 9 +++++++++ packages/hub-shim/dev/application-filter-smoke.ts | 3 +++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/adr/0010-platform-resolved-params.md b/docs/adr/0010-platform-resolved-params.md index b791a68..9733bde 100644 --- a/docs/adr/0010-platform-resolved-params.md +++ b/docs/adr/0010-platform-resolved-params.md @@ -54,10 +54,12 @@ serve different consumers and neither can do the other's job: Consequences, all verified in the shim prototype: -- Hub application ids are numeric (the harness's `APP_ID` parser requires a - uint), which makes them valid label values with no sanitization. One - numeric check covers both concerns: a run the harness would reject at - startup, and a create the apiserver would reject outright. +- Hub application ids must parse as a uint64 (`hub.ParseAppID` requires + it), capping them at 20 digits — inside the apiserver's 63-character + label-value limit, in a label-safe alphabet. One uint64-bounded check + covers both concerns: a run the harness would reject at startup, and a + create the apiserver would reject outright. A plain digits regex is not + that check — a 21-digit id passes it and overflows `ParseUint` anyway. - Runs created before the label are invisible to the selector. A filtered list is "runs we can prove belong to 42", not "every run that ever touched 42". Callers needing the old runs keep a `spec.env` fallback for diff --git a/packages/agentic-client/src/contract/index.ts b/packages/agentic-client/src/contract/index.ts index d662971..512cec7 100644 --- a/packages/agentic-client/src/contract/index.ts +++ b/packages/agentic-client/src/contract/index.ts @@ -461,10 +461,19 @@ export function invalidTargetBranchReason(branch: string): string | undefined { * also stamped as the {@link APPLICATION_LABEL} value, a create the * apiserver would reject outright. One check covers both. */ +/** What hub.ParseAppID accepts: strconv.ParseUint(s, 10, 64). */ +const UINT64_MAX = 18446744073709551615n; + export function invalidApplicationIdReason(id: string): string | undefined { if (!/^\d+$/.test(id)) { return `application id "${id}" is not numeric — the harness's APP_ID parser requires a uint Hub id`; } + // Digits alone are not enough: a 21-digit id passes the regex, then + // overflows the harness's ParseUint at startup. uint64 also caps ids at + // 20 characters, inside the apiserver's 63-char label-value limit. + if (BigInt(id) > UINT64_MAX) { + return `application id "${id}" overflows uint64 — the harness's APP_ID parser rejects it`; + } return undefined; } diff --git a/packages/hub-shim/dev/application-filter-smoke.ts b/packages/hub-shim/dev/application-filter-smoke.ts index 29f5947..5cc57ac 100644 --- a/packages/hub-shim/dev/application-filter-smoke.ts +++ b/packages/hub-shim/dev/application-filter-smoke.ts @@ -211,6 +211,9 @@ async function main(): Promise { ["non-run resource", "/api/agents?application=" + appA.id], ["empty value", "/api/agentruns?application="], ["non-numeric id", "/api/agentruns?application=not-a-number"], + // Digits alone must not pass: 21 nines overflows the harness's + // uint64 APP_ID parse, so the filter rejects what the harness would. + ["uint64 overflow", "/api/agentruns?application=999999999999999999999"], ]; for (const [label, path] of rejects) { const res = await call("GET", path);