From 32a6c09d7f1b2eb2a39a5e727cb3bcd5126f5fc0 Mon Sep 17 00:00:00 2001 From: Austin Kurpuis Date: Sat, 8 Aug 2026 08:32:15 -0700 Subject: [PATCH 1/3] Prove the Temporal engine actually runs a bridged pod agent end-to-end The chart annotation fix (PR #204) was necessary but not sufficient: no Temporal server exists in the e2e minikube profile, and the engine's own config had several unset wires that silently degrade every turn to a conversational bare-answer or a link-required park, never reaching a real bridged episode. Fixed all of them and added the e2e spec that proves it: - e2e/manifests/temporal-dev-server.yaml: a single-node dev-mode Temporal server (temporalio/admin-tools' `temporal server start-dev`), applied by e2e/scripts/up.sh before the release deploys. - charts/agent-controller/values-e2e.yaml: wires temporal-engine.nats.url, .qdrant.host (retrieval was otherwise disabled entirely), .gateway.identity.defaultSubject/defaultRoles (every internal call from agent-orchestrator arrives tokenless, so an empty Caller.Subject made every turn bare-answer regardless of capability/retrieval/forcedAgentId), and .identityLink.gatewayUrl (without it the worker uses an in-memory dev fake, completely disconnected from this suite's real credential seeding). - engines/temporal/internal/identitylink/identitylink.go: the claude-remote provider's `/claude-auth/api/token?mode=login` response uses a `credentialsJson` field, not `token` -- the client only ever read `token`, so a real, retrievable credential was silently treated as absent. - controllers/core-controller/internal/controller/agentrun_controller.go: the Temporal engine names AgentRun CRs "agentrun--" itself; prefixing "agentrun-" onto that again produced a Job name (and the API server's auto-added job-name pod-template label, which reuses it verbatim) over the 63-byte label limit, so the reconciler could never create a Job for ANY AgentRun this engine launches. - e2e/specs/bridged-agent-workflow.e2e.ts: drives the same webhook path happy-path.e2e.ts does, then asks Temporal itself (not a log line) for a completed BridgedAgentWorkflow execution -- verified passing. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01HctRMtdZixptEZeMebhADq --- charts/agent-controller/values-e2e.yaml | 91 ++++++++ .../controller/agentrun_controller.go | 11 +- e2e/manifests/temporal-dev-server.yaml | 85 ++++++++ e2e/scripts/up.sh | 12 ++ e2e/specs/bridged-agent-workflow.e2e.ts | 201 ++++++++++++++++++ .../internal/identitylink/identitylink.go | 11 + 6 files changed, 410 insertions(+), 1 deletion(-) create mode 100644 e2e/manifests/temporal-dev-server.yaml create mode 100644 e2e/specs/bridged-agent-workflow.e2e.ts diff --git a/charts/agent-controller/values-e2e.yaml b/charts/agent-controller/values-e2e.yaml index 3b70e60..08ea425 100644 --- a/charts/agent-controller/values-e2e.yaml +++ b/charts/agent-controller/values-e2e.yaml @@ -17,6 +17,97 @@ # genuinely real key the stack needs (OPENAI_API_KEY, for the orchestrator's # planner) is created out-of-band by a human — see that script. +temporal-engine: + # Points the worker/gateway at `e2e/manifests/temporal-dev-server.yaml` + # (applied by `e2e/scripts/up.sh` before this release is deployed) instead of + # the production default (a real `temporalio/helm-charts` release's frontend + # Service, `temporal-frontend.temporal.svc:7233`) -- there is no such cluster + # here, and the chart's own default address is unreachable in a hermetic + # minikube profile. + temporal: + address: temporal-dev-server.controller-agent.svc:7233 + namespace: default + llm: + # The subchart's own default (`agent-controller-agent-orchestrator`) + # assumes the `agent-orchestrator` subchart at its OWN release-name-derived + # secret name, but that subchart sets `fullnameOverride: agent-orchestrator` + # (`charts/agent-controller/values.yaml`), so its real Secret is + # `agent-orchestrator-secrets` -- the default name doesn't exist in ANY + # profile, e2e or otherwise, and the worker/gateway Pods CreateContainer- + # ConfigError on first boot without this override. + secretName: agent-orchestrator-secrets + callback: + # Same `fullnameOverride` gap as `llm.secretName` above, for the gateway's + # HMAC callback secret -- `agent-orchestrator-secrets` already carries + # `AGENT_CALLBACK_SECRET` (matching `callback.secretKey`'s default), since + # that's the same value agent-orchestrator's own tool-callback path signs + # with. + secretName: agent-orchestrator-secrets + nats: + # Empty by default -- `charts/agent-controller/charts/temporal-engine`'s own + # comment says this "disables bridged pod agents entirely, leaving the + # declarative loop and checkpoint-resume step tools", and the worker logs + # it plainly at startup ("AGENT_NATS_URL not set; bridged pod agents + # disabled"). This is the exact NATS the orchestrator itself is wired to + # (`agent-orchestrator.natsUrl` in values-minikube-demo.yaml derives the + # same Service) -- both engines have to speak to the same broker, since a + # bridged agent's Job publishes to AgentRun-id-keyed subjects regardless of + # which engine is driving the episode. + url: nats://agent-controller-nats.controller-agent.svc.cluster.local:4222 + qdrant: + # Empty by default -- and every catalog-dependent activity (IntegrationRoute + # matching, Skill/Agent/Tool retrieval, delegation) needs it: without a + # Qdrant host, the worker logs "QDRANT_HOST not set; retrieval activities + # disabled" and every turn falls through the capability-need gate into a + # bare conversational answer, NEVER reaching delegateToAgent -- so no + # Agent, bridged or otherwise, is ever run. Reuses agent-orchestrator's own + # Qdrant instance (already deployed by this same release) rather than + # standing up a second one; `collectionPrefix` keeps its collections + # separate from the orchestrator's own (their payload schemas differ). + host: agent-controller-qdrant + port: 6334 + collectionPrefix: te- + gateway: + identity: + # The Go gateway resolves ITS OWN caller identity from a bearer token -- + # entirely separate from agent-orchestrator's own `config.staticIdentities` + # below, despite the identical JSON shape. But agent-orchestrator's + # internal hop to this gateway (`TemporalEngine`, apps/agent-orchestrator/ + # src/engine/temporal-engine.ts) sends NO Authorization header at all + # unless `AGENT_TEMPORAL_ENGINE_TOKEN` is set, which nothing in this + # chart ever populates (its own secretKeyRef is `optional: true` against + # a key that doesn't exist). So EVERY call arrives tokenless, resolving + # to an EMPTY Caller.Subject -- and `runAgentTurn` + # (engines/temporal/internal/temporal/workflows/agentloop.go) treats an + # empty subject as "answer bare, skip the catalog entirely", + # unconditionally, regardless of the capability gate, retrieval, or a + # route's `forcedAgentId`. Without this, NOTHING ever delegates to any + # Agent under the Temporal engine -- not a bridged-agent-specific gap, + # a total one. `defaultSubject`/`defaultRoles` are exactly the + # documented lever for a tokenless caller (main.go: "give tokenless + # callers an identity — leave unset to fail closed to no + # capabilities"), matching the role agent-orchestrator's own shared + # `client-integration-gateway` subject plays for every webhook-driven + # turn today. + defaultSubject: client-integration-gateway + defaultRoles: reader,writer + identityLink: + # Without this the worker logs "IDENTITY_LINK_GATEWAY_URL not set; using + # the in-memory identity-link fake (dev only)" -- a completely separate, + # unseedable identity-link store from the one this e2e suite actually + # seeds into (e2e/support/credential-store.ts writes real Kubernetes + # Secrets that integration-gateway's identity-link API reads). Every + # Agent requiring `identityProviders` therefore parks on "link-required" + # forever under the fake, regardless of anything seeded. Mirrors + # agent-orchestrator's own `identityLink.gatewayUrl` above at the + # identical Service address, and reuses the same shared secret VALUE + # bootstrap-secrets.sh already wrote for the reverse direction (its own + # key name differs -- IDENTITY_LINK_GATEWAY_TOKEN vs + # GATEWAY_IDENTITY_LINK_TOKEN -- but both hold the one value both sides + # of this handshake must agree on). + gatewayUrl: "http://agent-controller-integration-gateway:8090" + tokenSecretName: e2e-integration-gateway-secrets + integration-gateway: # The whole point of this overlay. Off in every other profile. enabled: true diff --git a/controllers/core-controller/internal/controller/agentrun_controller.go b/controllers/core-controller/internal/controller/agentrun_controller.go index 4130cd8..a57a530 100644 --- a/controllers/core-controller/internal/controller/agentrun_controller.go +++ b/controllers/core-controller/internal/controller/agentrun_controller.go @@ -150,7 +150,16 @@ func (r *AgentRunReconciler) createJob(ctx context.Context, run *toolv1alpha1.Ag } job, err := buildRunJob(runJobParams{ - jobName: fmt.Sprintf("agentrun-%s", run.Name), + // run.Name IS the Job name directly, not "agentrun-"+run.Name: the + // Temporal engine (engines/temporal's BridgedAgentWorkflow) names + // AgentRun CRs "agentrun--" itself, so prepending this + // prefix again produced a Job name (and therefore the API server's + // auto-added spec.template.labels["job-name"], which reuses the Job's + // own name verbatim with no truncation) exceeding the 63-byte label + // limit -- the exact failure mode this fixes. Job lookup is by owner + // reference (`.Owns(&batchv1.Job{})` below), never by reconstructing + // this name, so dropping the prefix changes nothing else. + jobName: run.Name, namespace: run.Namespace, annotations: sessionIDAnnotations(run.Annotations), labels: map[string]string{ diff --git a/e2e/manifests/temporal-dev-server.yaml b/e2e/manifests/temporal-dev-server.yaml new file mode 100644 index 0000000..be06f84 --- /dev/null +++ b/e2e/manifests/temporal-dev-server.yaml @@ -0,0 +1,85 @@ +# temporal-dev-server.yaml — a single-node Temporal server for the e2e stack. +# +# `charts/agent-controller/values.yaml` defaults `temporal-engine.enabled: true` +# and `agent-orchestrator.config.agentEngine: temporal` (docs/adr/0036), so every +# e2e run now routes agent turns through the Temporal engine's worker/gateway — +# but neither the temporal-engine subchart nor `e2e/scripts/up.sh` ever bundled +# an actual Temporal server ("Assumes a reachable Temporal cluster; no server is +# bundled", `charts/agent-controller/values.yaml`). Without one, the engine's +# worker/gateway crashloop trying to reach a frontend that doesn't exist, and +# nothing in the suite ever proves an Agent actually took the +# `BridgedAgentWorkflow` path. +# +# Uses the official `temporalio/admin-tools` image's bundled `temporal` CLI +# (`temporal server start-dev`) rather than the full multi-service Helm chart +# (Cassandra/Elasticsearch, etc.) — a single in-memory dev-mode server is all a +# hermetic, throwaway e2e cluster needs, and it starts in seconds. `--ip +# 0.0.0.0` is required: the CLI binds the frontend to `localhost` by default, +# which is unreachable from any other Pod. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: temporal-dev-server + labels: + app: temporal-dev-server +spec: + replicas: 1 + selector: + matchLabels: + app: temporal-dev-server + template: + metadata: + labels: + app: temporal-dev-server + spec: + containers: + - name: temporal + image: temporalio/admin-tools:latest + command: + - temporal + - server + - start-dev + - --ip=0.0.0.0 + - --port=7233 + - --http-port=7243 + - --ui-port=8233 + ports: + - name: grpc + containerPort: 7233 + - name: http + containerPort: 7243 + - name: ui + containerPort: 8233 + readinessProbe: + tcpSocket: + port: 7233 + initialDelaySeconds: 3 + periodSeconds: 2 + failureThreshold: 30 + resources: + requests: + cpu: "100m" + memory: "256Mi" + limits: + cpu: "1" + memory: "512Mi" +--- +apiVersion: v1 +kind: Service +metadata: + name: temporal-dev-server + labels: + app: temporal-dev-server +spec: + selector: + app: temporal-dev-server + ports: + - name: grpc + port: 7233 + targetPort: 7233 + - name: http + port: 7243 + targetPort: 7243 + - name: ui + port: 8233 + targetPort: 8233 diff --git a/e2e/scripts/up.sh b/e2e/scripts/up.sh index e924e07..a54f580 100755 --- a/e2e/scripts/up.sh +++ b/e2e/scripts/up.sh @@ -60,6 +60,18 @@ echo " ✓ $(ls "$REPO_ROOT"/controllers/core-controller/config/crd/bases/*.yam step "Creating throwaway secrets..." "$REPO_ROOT/e2e/scripts/bootstrap-secrets.sh" +step "Deploying the Temporal dev server (e2e/manifests/temporal-dev-server.yaml)..." +# The chart defaults `temporal-engine.enabled: true` + +# `agent-orchestrator.config.agentEngine: temporal` (docs/adr/0036), so every +# turn now routes through the Temporal engine's worker/gateway -- but no +# subchart bundles an actual Temporal server. Without one reachable BEFORE the +# release below, the worker/gateway Pods crashloop on their first connection +# attempt and every agent turn in the suite hangs. Applied idempotently, same +# as the CRDs above. +kubectl apply -n "$NS" -f "$REPO_ROOT/e2e/manifests/temporal-dev-server.yaml" >/dev/null +kubectl -n "$NS" wait --for=condition=Available --timeout=120s deploy/temporal-dev-server >/dev/null +echo " ✓ temporal-dev-server" + step "Adopting hand-created objects into Helm..." # dev-up.sh creates these ServiceAccounts with `kubectl create serviceaccount`, # but community-components' templates also declare them. Helm refuses to adopt diff --git a/e2e/specs/bridged-agent-workflow.e2e.ts b/e2e/specs/bridged-agent-workflow.e2e.ts new file mode 100644 index 0000000..beb95dd --- /dev/null +++ b/e2e/specs/bridged-agent-workflow.e2e.ts @@ -0,0 +1,201 @@ +import { execFile } from "node:child_process"; +import { connect } from "node:net"; +import { promisify } from "node:util"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { requireMinikubeContext } from "../support/guard.js"; +import { agentRunsSince, cleanupAgentRunsSince, waitFor } from "../support/k8s.js"; +import { withPortForward } from "../support/k8s.js"; +import { issueLabeledPayload, postGithubWebhook } from "../support/webhook.js"; +import { fakeGithubRequests, resetFakeGithub, webhookSecret } from "../support/fixtures.js"; +import { seedAllClaudeCredentials } from "../support/credential-store.js"; + +/** + * `support/k8s.ts`'s own `withPortForward` gates readiness on a plain HTTP + * `fetch()`, which is the right probe for every OTHER service this suite + * forwards to but wrong for `temporal-dev-server:7233` -- Temporal's frontend + * speaks raw gRPC/HTTP2, which never answers an HTTP/1.1 GET, so that probe + * timed out on every attempt for the full 30s window regardless of whether + * the forward was actually up (confirmed: the identical forward worked fine + * against the `temporal` CLI the whole time this was "failing"). A bare TCP + * connect is the correct readiness signal for a gRPC port. + */ +async function withTemporalPortForward(localPort: number, body: () => Promise): Promise { + requireMinikubeContext(); + const child = execFile("kubectl", ["-n", "controller-agent", "port-forward", "svc/temporal-dev-server", `${localPort}:7233`]); + let exited: string | undefined; + child.on("exit", (code, signal) => { + exited = `kubectl port-forward svc/temporal-dev-server exited (code ${code ?? "null"}, signal ${signal ?? "null"})`; + }); + try { + await waitFor( + "port-forward to temporal-dev-server:7233 to accept connections", + () => + new Promise((resolve) => { + const socket = connect({ host: "127.0.0.1", port: localPort, timeout: 1_000 }); + socket.once("connect", () => { + socket.destroy(); + resolve(true); + }); + socket.once("error", () => resolve(undefined)); + socket.once("timeout", () => { + socket.destroy(); + resolve(undefined); + }); + }), + { timeoutMs: 30_000, intervalMs: 500 }, + ); + return await body(); + } finally { + if (!exited) child.kill(); + } +} + +/** + * Proves the Temporal engine (docs/adr/0036) actually routes a bridged pod + * agent (`claude-code-swe-agent`, `opencode-swe-agent`, and their e2e stand-in + * `stub-agent`) through `BridgedAgentWorkflow` -- not the declarative + * `AgentWorkflow` those agents were silently falling back to. + * + * That fallback was a real, reproduced production bug: `stub-agent`/ + * `claude-code-swe-agent` missing the `durable-agents.dev/bridged` chart + * annotation left `agentWorkflowNameFor` (engines/temporal/internal/temporal/ + * workflows/agent_workflow.go) defaulting to the declarative loop, which has + * no real tools at all -- the planner then guessed at tool names ("gh", + * "gh_repo_clone", "call_tool") and every guess was refused. The annotation + * fix alone is necessary but not sufficient: getting a REAL bridged episode to + * run end-to-end on this suite's minikube profile also needed a dev-mode + * Temporal server (none is bundled -- e2e/manifests/temporal-dev-server.yaml) + * and several engine-config wires that had no default in a hermetic profile + * (temporal-engine.nats.url, .qdrant.host, .gateway.identity.defaultSubject/ + * defaultRoles, .identityLink.gatewayUrl -- all in values-e2e.yaml), plus a + * code fix for a claude-remote credential response-shape mismatch + * (engines/temporal/internal/identitylink/identitylink.go) and a Job-name + * double-prefix bug that broke Job creation for ANY AgentRun this engine + * launches (controllers/core-controller/internal/controller/ + * agentrun_controller.go). + * + * This spec is the actual proof point for all of that: it drives the same + * webhook path `happy-path.e2e.ts` does, then asks Temporal itself (not a log + * line, not an inferred side effect) whether a `BridgedAgentWorkflow` + * execution completed for this run. + */ + +const execFileAsync = promisify(execFile); + +requireMinikubeContext(); + +const GATEWAY_PORT = 18092; +const TEMPORAL_PORT = 17234; +// Must be one of values-e2e.yaml's `integration-gateway.config.githubIdentities` +// entries -- an unlisted sender resolves no identity and the turn is dropped +// before it ever reaches the orchestrator, silently (no error, no AgentRun). +const SENDER = "e2e-other-user"; +const OWNER = "e2e-bridged-org"; +const REPO = "e2e-bridged-repo"; +const STUB_REPLY_MARKER = "stub-agent-reply"; + +/** + * The Temporal engine's Go gateway resolves every internal, tokenless call + * from agent-orchestrator to this subject (`gateway.identity.defaultSubject`, + * values-e2e.yaml) -- there is no per-end-user token on this hop regardless of + * which GitHub user triggered the webhook, so this is the subject a credential + * must be seeded under for the engine's own identity-link gate to resolve it. + */ +const ENGINE_CALLER_SUBJECT = "client-integration-gateway"; + +describe("Temporal engine: a bridged pod agent actually runs as BridgedAgentWorkflow", () => { + let secret: string; + let suiteStartedAt: Date; + + beforeAll(async () => { + suiteStartedAt = new Date(); + secret = await webhookSecret(); + await resetFakeGithub(); + // Both providers stub-agent's Agent CR declares in this cluster (mirrors + // claude-code-swe-agent's own identityProviders) -- see this suite's + // credential-store.ts doc comment for why seeding is the only hermetic way + // to get an authorized (not link-required) verdict. + await seedAllClaudeCredentials(ENGINE_CALLER_SUBJECT); + }); + + afterAll(async () => { + await cleanupAgentRunsSince(suiteStartedAt); + }); + + it("launches a real BridgedAgentWorkflow execution for the bridged agent an IntegrationRoute names", async () => { + const startedAt = new Date(); + const issueNumber = Date.now() % 100000; + + const status = await withPortForward("agent-controller-integration-gateway", 8090, GATEWAY_PORT, async (baseUrl) => { + const res = await postGithubWebhook( + baseUrl, + "issues", + issueLabeledPayload({ + owner: OWNER, + repo: REPO, + issueNumber, + label: "ai-triage", + senderLogin: SENDER, + }), + secret, + ); + return res.status; + }); + expect(status).toBeGreaterThanOrEqual(200); + expect(status).toBeLessThan(300); + + // External proof: the run reached a real pod, which posted the comment + // back through the (fake) GitHub API -- same assertion happy-path.e2e.ts + // makes, kept here so this spec stands on its own. + const comment = await waitFor( + "the stub agent's reply to be posted back to the issue", + async () => { + const posted = (await fakeGithubRequests()).filter( + (r) => r.method === "POST" && r.path === `/repos/${OWNER}/${REPO}/issues/${issueNumber}/comments`, + ); + return posted.length > 0 ? posted[posted.length - 1] : undefined; + }, + { timeoutMs: 420_000 }, + ); + expect(comment?.body).toContain(STUB_REPLY_MARKER); + + // Mechanism proof: Temporal itself, not a log line, says a + // BridgedAgentWorkflow execution actually ran and completed for the + // route's target Agent (`stub-agent`, per the `github-issue-labeled- + // triage` IntegrationRoute this webhook matches) -- the only way that's + // true is if `agentWorkflowNameFor` resolved the bridged branch, + // LaunchAgentRun actually created the Job (which needed the + // core-controller Job-name fix), and the pod ran to completion. + const run = (await agentRunsSince(startedAt))[0]; + expect(run, "an AgentRun should have been created for the labeled issue").toBeTruthy(); + + const completed = await withTemporalPortForward(TEMPORAL_PORT, async () => { + return waitFor( + "a completed BridgedAgentWorkflow execution for stub-agent", + async () => { + const { stdout } = await execFileAsync("temporal", [ + "workflow", + "list", + "--address", + `localhost:${TEMPORAL_PORT}`, + "--namespace", + "default", + "--query", + `WorkflowType='BridgedAgentWorkflow' and ExecutionStatus='Completed'`, + "-o", + "json", + ]); + const executions = JSON.parse(stdout || "[]") as Array<{ + execution: { workflowId: string }; + startTime: string; + }>; + return executions.find( + (e) => e.execution.workflowId.startsWith("agent-stub-agent-") && new Date(e.startTime) >= startedAt, + ); + }, + { timeoutMs: 60_000 }, + ); + }); + expect(completed).toBeTruthy(); + }); +}); diff --git a/engines/temporal/internal/identitylink/identitylink.go b/engines/temporal/internal/identitylink/identitylink.go index 0b3cb1a..1e9386b 100644 --- a/engines/temporal/internal/identitylink/identitylink.go +++ b/engines/temporal/internal/identitylink/identitylink.go @@ -58,6 +58,14 @@ type Token struct { // or returned into workflow state; see authz.Service for the discipline. Value string `json:"token"` GitHubLogin string `json:"githubLogin,omitempty"` + // CredentialsJSON is `/claude-auth/api/token?mode=login`'s own field name + // for the same credential material `Value` holds for every other + // provider/mode -- a full Claude Code login blob, not a bearer token, so + // the gateway's response shape differs from `token`. Never populated for + // anything except claude-remote; folded into Value below rather than + // exposed here, since every caller of Token() reads Value regardless of + // provider. + CredentialsJSON string `json:"credentialsJson,omitempty"` } // Poll statuses for a device flow. @@ -279,6 +287,9 @@ func (c *Client) Token(ctx context.Context, provider, subject string) (*Token, e if err != nil { return nil, err } + if out.Value == "" { + out.Value = out.CredentialsJSON + } if status == http.StatusNotFound || out.Value == "" { return nil, nil } From 638f7283c7794e4ef8f9580a2337c0e4b891fbaf Mon Sep 17 00:00:00 2001 From: Austin Kurpuis Date: Sat, 8 Aug 2026 09:10:11 -0700 Subject: [PATCH 2/3] Give bridged-agent-workflow.e2e.ts its own local gateway port It was reusing 18092, the same local port identity-keying.e2e.ts already binds for its own gateway port-forward -- a collision waiting to bite the full suite even though file-level serial execution mostly hides it today. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01HctRMtdZixptEZeMebhADq --- e2e/specs/bridged-agent-workflow.e2e.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e/specs/bridged-agent-workflow.e2e.ts b/e2e/specs/bridged-agent-workflow.e2e.ts index beb95dd..e729b03 100644 --- a/e2e/specs/bridged-agent-workflow.e2e.ts +++ b/e2e/specs/bridged-agent-workflow.e2e.ts @@ -84,7 +84,8 @@ const execFileAsync = promisify(execFile); requireMinikubeContext(); -const GATEWAY_PORT = 18092; +// 18090/18091/18092 belong to happy-path/resilience/identity-keying respectively. +const GATEWAY_PORT = 18093; const TEMPORAL_PORT = 17234; // Must be one of values-e2e.yaml's `integration-gateway.config.githubIdentities` // entries -- an unlisted sender resolves no identity and the turn is dropped From 9748461ea46e7eb84098cb32aacf5ae11be80f84 Mon Sep 17 00:00:00 2001 From: Austin Kurpuis Date: Fri, 14 Aug 2026 19:18:43 -0700 Subject: [PATCH 3/3] Wire real per-user identity into the Temporal engine, fix credential adoption and message wording Chasing this PR's remaining test-plan item (a full e2e regression pass) surfaced several more independent bugs, all now fixed: - The Temporal engine gateway never forwarded/verified a per-request caller identity for chat turns -- every internal hop from agent-orchestrator resolved to the SAME shared default subject regardless of which human was chatting, so credential convergence (ADR 0031) could never work under this engine. Added a signed "caller identity" assertion (mirrors the existing sender-assertion mechanism, reuses the same secret) carrying the caller's real subject/roles plus a PerUser flag -- true only for a genuinely per-request identity (Open WebUI's forwarded-user JWT), never a shared token, since that flag gates the GitHub-link-based principal upgrade that credential sharing depends on. Scoped deliberately to the forwarded-user-JWT path only, after an initial version regressed webhook-driven turns by also resolving (and overriding the correct subject with) integration-gateway's own shared service token. - identitylink.Client.Rekey parsed a boolean `moved` field that never existed on the wire -- the real response is `{status: "moved"|"not-found"|"occupied"}` -- so it silently read false for every response, making credential adoption always appear to fail and fall through to re-prompting the caller. - authz.composeLinkRequired's multi-link message wording and the "claude-remote" provider label had drifted from the original TypeScript implementation (missing the "N accounts" count, wrong parenthesized label), breaking round-trip parity with upstream copy. - bridged-agent-workflow.e2e.ts seeded credentials at a stale placeholder subject that only "worked" before principal resolution was fixed; updated to the correct canonical-principal subject. Verified: identity-keying.e2e.ts's full 15 tests pass (previously hung indefinitely), caller-tools.e2e.ts 18/18, happy-path/bridged-agent- workflow/chat-harness/waitfor-guard all pass individually. resilience.e2e.ts has 2 pre-existing failures (a gRPC-level stream cancellation surfacing instead of the workflow's own idle-timeout message) unrelated to any change in this session -- left as a known follow-up, not fabricated as passing. Co-Authored-By: Claude Sonnet 5 --- .../src/engine/temporal-engine.ts | 71 ++++++++++++++ apps/agent-orchestrator/src/index.ts | 6 ++ .../src/rbac/caller-identity-assertion.ts | 63 ++++++++++++ charts/agent-controller/Chart.lock | 6 +- charts/agent-controller/values-e2e.yaml | 11 +++ e2e/specs/bridged-agent-workflow.e2e.ts | 16 +-- e2e/support/k8s.ts | 2 +- engines/temporal/internal/authz/authz.go | 39 +++++--- engines/temporal/internal/authz/authz_test.go | 2 +- engines/temporal/internal/gateway/invoke.go | 55 ++++++++++- .../internal/identitylink/identitylink.go | 10 +- .../internal/identitylink/rekey_test.go | 69 +++++++++++++ .../rbac/caller_identity_assertion.go | 98 +++++++++++++++++++ .../rbac/caller_identity_assertion_test.go | 64 ++++++++++++ 14 files changed, 485 insertions(+), 27 deletions(-) create mode 100644 apps/agent-orchestrator/src/rbac/caller-identity-assertion.ts create mode 100644 engines/temporal/internal/identitylink/rekey_test.go create mode 100644 engines/temporal/internal/rbac/caller_identity_assertion.go create mode 100644 engines/temporal/internal/rbac/caller_identity_assertion_test.go diff --git a/apps/agent-orchestrator/src/engine/temporal-engine.ts b/apps/agent-orchestrator/src/engine/temporal-engine.ts index c5b3dd9..4c45101 100644 --- a/apps/agent-orchestrator/src/engine/temporal-engine.ts +++ b/apps/agent-orchestrator/src/engine/temporal-engine.ts @@ -1,6 +1,9 @@ import { SENDER_ASSERTION_HEADER, mintSenderAssertion } from "../rbac/sender-assertion.js"; +import { CALLER_IDENTITY_HEADER, mintCallerIdentityAssertion } from "../rbac/caller-identity-assertion.js"; import type { AgentGraphInput, AgentGraphLike } from "../server.js"; import type { AgentState } from "../agent/graph.js"; +import type { IdentityResolver } from "../rbac/types.js"; +import { CALLER_TOOL_ID_PREFIX } from "../caller-tools/types.js"; /** * Runs a turn on the Temporal engine (`engines/temporal`) instead of the @@ -48,6 +51,21 @@ export interface TemporalEngineOptions { * sharing degrades — the same documented weaker mode as the gateway hop. */ senderAssertionSecret?: string; + /** + * Resolves Open WebUI's per-request signed user JWT into a real per-user + * identity (the SAME resolver the LangGraph engine's `resolveIdentity` node + * uses for `state.forwardedUserToken`) -- needed because the gateway + * otherwise only ever sees ONE shared service identity for every internal + * hop, which would collapse every Open WebUI user onto that one subject. + * Deliberately NOT a general authToken-based resolver: every OTHER caller of + * this process's own /invoke (a webhook relay, a static-token programmatic + * caller) shares one token that says nothing about who is actually asking, + * and forwarding an identity resolved from it would override the subject a + * webhook turn's own senderLogin/sender-assertion channel already handles. + * Absent -> every chat turn on this engine resolves the gateway's own + * default/bearer identity, same as before this existed. + */ + forwardedUserIdentityResolver?: IdentityResolver; /** How long to keep polling one turn before giving up. */ timeoutMs?: number; /** Injectable for tests; defaults to the global fetch. */ @@ -137,6 +155,34 @@ export class TemporalEngine implements AgentGraphLike { headers[SENDER_ASSERTION_HEADER] = mintSenderAssertion(this.options.senderAssertionSecret, input.senderLogin); } + // This process's OWN per-request identity resolution (OIDC/static/ + // forwarded-user-JWT), signed across the hop -- without this, every + // caller of /invoke resolves to the SAME gateway-default/bearer subject + // regardless of which human triggered the turn, which is exactly the + // collapsed-identity bug ADR 0030 fixed for webhooks. Mirrors graph.ts's + // resolveIdentity node: prefer the forwarded-user JWT over the shared + // static authToken when both are available. + // Scoped to the forwarded-user-JWT path ONLY, deliberately -- not a + // general "resolve identity somehow" fallback. `input.authToken` alone is + // ONE value every caller of this process's own /invoke shares (Open WebUI's + // shared bearer token, integration-gateway's static service token for a + // relayed webhook turn, etc.), so resolving it says nothing about who is + // actually asking and would override the correctly-defaulted gateway + // subject that a webhook turn's OWN senderLogin/sender-assertion channel + // already handles -- regressing every webhook-driven turn's credential + // resolution the moment this identity resolves to anything at all. + if (this.options.senderAssertionSecret && input.forwardedUserToken && this.options.forwardedUserIdentityResolver) { + const identity = await this.options.forwardedUserIdentityResolver.resolve(input.forwardedUserToken); + if (identity) { + headers[CALLER_IDENTITY_HEADER] = mintCallerIdentityAssertion( + this.options.senderAssertionSecret, + identity.subject, + identity.roles, + true, + ); + } + } + const res = await this.fetchImpl(`${this.baseUrl}/invoke`, { method: "POST", headers, @@ -148,6 +194,31 @@ export class TemporalEngine implements AgentGraphLike { // still re-resolves it under the caller's own roles. ...(input.forcedSkillId ? { forcedSkillId: input.forcedSkillId } : {}), ...(input.forcedAgentId ? { forcedAgentId: input.forcedAgentId } : {}), + // Already resolved, validated and top-K-ranked by this process's own + // handleChat pipeline (ADR 0035) before invoke() is ever called -- + // the engine's /invoke takes the resolved Descriptor shape (each + // ToolDescriptor's nested `callerTool`), not a raw OpenAI tools array, + // and re-ranks nothing. Omitting this silently drops every caller + // tool for any turn routed through this engine. + ...(input.callerTools?.length + ? { callerTools: input.callerTools.map((t) => t.callerTool).filter(Boolean) } + : {}), + ...(input.callerToolChoiceRequired ? { callerToolRequired: true } : {}), + // Prior client-executed calls (ADR 0035 resume), the same + // strip-the-namespace transform as everywhere else a caller-tool id + // crosses back out of the `caller:` namespace -- the engine's own + // callertools.ID re-adds it, so forwarding it prefixed would double it. + ...(input.actionHistory?.length + ? { + priorCallerToolCalls: input.actionHistory.map((call) => ({ + name: call.toolId.startsWith(CALLER_TOOL_ID_PREFIX) + ? call.toolId.slice(CALLER_TOOL_ID_PREFIX.length) + : call.toolId, + arguments: call.toolArgs, + result: call.result, + })), + } + : {}), }), }); if (!res.ok) { diff --git a/apps/agent-orchestrator/src/index.ts b/apps/agent-orchestrator/src/index.ts index 985b2b5..ca7d69e 100644 --- a/apps/agent-orchestrator/src/index.ts +++ b/apps/agent-orchestrator/src/index.ts @@ -608,6 +608,12 @@ async function main(): Promise { // Reuses the assertion contract rather than trusting a login on an // internal hop -- see TemporalEngine's own note. ...(config.senderAssertionSecret ? { senderAssertionSecret: config.senderAssertionSecret } : {}), + // Same forwarded-user resolver `graph`'s resolveIdentity node uses -- + // without it, every CHAT turn on this engine collapses onto the + // gateway's shared default/bearer identity regardless of which human is + // chatting. Scoped to this one resolver deliberately -- see + // TemporalEngineOptions.forwardedUserIdentityResolver's doc comment. + ...(forwardedUserIdentityResolver ? { forwardedUserIdentityResolver } : {}), timeoutMs: config.agentRunTimeoutSeconds * 1_000, }); console.log(`agent engine: temporal (${config.temporalEngineUrl})`); diff --git a/apps/agent-orchestrator/src/rbac/caller-identity-assertion.ts b/apps/agent-orchestrator/src/rbac/caller-identity-assertion.ts new file mode 100644 index 0000000..cf0754e --- /dev/null +++ b/apps/agent-orchestrator/src/rbac/caller-identity-assertion.ts @@ -0,0 +1,63 @@ +import { createHmac } from "node:crypto"; + +/** + * Header carrying this process's signed claim about which per-request + * identity it already resolved for a chat/invoke turn -- its OWN + * OIDC/static/forwarded-user-JWT resolution, not the Temporal engine + * gateway's bearer-token map. + * + * Every internal hop to that gateway otherwise authenticates with ONE shared + * service token (or none), so every caller resolves to the same subject + * regardless of which human is actually chatting -- collapsing every Open + * WebUI user onto one identity, the same bug class ADR 0030 fixed for + * webhooks via `SENDER_ASSERTION_HEADER`. This is that fix generalized: a + * resolved SUBJECT (and its roles) rather than a GitHub login, signed for the + * same reason -- an unsigned field would let anything holding the gateway's + * token name an arbitrary subject. + */ +export const CALLER_IDENTITY_HEADER = "x-gateway-caller-identity"; + +const DEFAULT_TTL_SECONDS = 300; + +interface CallerIdentityPayload { + subject: string; + roles: string[]; + /** + * True only when this subject came from a per-request signed identity + * (Open WebUI's forwarded-user JWT, or real OIDC) -- never from a shared + * static/bearer token, which can resolve successfully yet still be one + * value every caller presents. Gates whether the engine will look up a + * GitHub-link-based principal upgrade at all (ADR 0030 §6/0031): doing that + * for a shared subject would let whoever links first share their + * credentials with everyone else who happens to authenticate the same way. + */ + perUser: boolean; + /** Unix seconds. */ + exp: number; +} + +function b64url(buf: Buffer): string { + return buf.toString("base64url"); +} + +function sign(secret: string, payloadB64: string): string { + return b64url(createHmac("sha256", secret).update(payloadB64).digest()); +} + +/** + * Mints `.` for a resolved subject/roles pair. Same HMAC + * scheme as `mintSenderAssertion`, deliberately a separate payload/header: + * this asserts a resolved caller identity, not a GitHub login. + */ +export function mintCallerIdentityAssertion( + secret: string, + subject: string, + roles: string[], + perUser: boolean, + ttlSeconds = DEFAULT_TTL_SECONDS, + now = Date.now(), +): string { + const payload: CallerIdentityPayload = { subject, roles, perUser, exp: Math.floor(now / 1000) + ttlSeconds }; + const payloadB64 = b64url(Buffer.from(JSON.stringify(payload), "utf8")); + return `${payloadB64}.${sign(secret, payloadB64)}`; +} diff --git a/charts/agent-controller/Chart.lock b/charts/agent-controller/Chart.lock index 21bd2b1..1b78bbe 100644 --- a/charts/agent-controller/Chart.lock +++ b/charts/agent-controller/Chart.lock @@ -7,7 +7,7 @@ dependencies: version: 0.1.0 - name: temporal-engine repository: file://charts/temporal-engine - version: 0.1.1 + version: 0.2.0 - name: integration-gateway repository: file://charts/integration-gateway version: 0.1.0 @@ -17,5 +17,5 @@ dependencies: - name: nats repository: https://nats-io.github.io/k8s/helm/charts/ version: 2.14.2 -digest: sha256:571058ff9cac13b57a9e36b07e9a866cca84904a0411bffde4b556f3ea1cea7b -generated: "2026-08-06T18:08:32.066566-07:00" +digest: sha256:5dd4996b45156a10e81616ca4293ed907d5404250dc5896c7ade4eb85014e5dd +generated: "2026-08-14T16:05:33.799016-07:00" diff --git a/charts/agent-controller/values-e2e.yaml b/charts/agent-controller/values-e2e.yaml index 08ea425..26ee372 100644 --- a/charts/agent-controller/values-e2e.yaml +++ b/charts/agent-controller/values-e2e.yaml @@ -68,6 +68,17 @@ temporal-engine: port: 6334 collectionPrefix: te- gateway: + senderAssertion: + # Without this, `GATEWAY_SENDER_ASSERTION_SECRET` is unset on the + # Temporal engine gateway, so it can never verify the signed assertion + # header agent-orchestrator's TemporalEngine sends (temporal-engine.ts: + # "the sender login travels as a SIGNED assertion, not a body field" -- + # it never falls back to an unsigned body field for this hop). The + # gateway silently resolves an empty sender login instead, which is why + # `AGENT_ACTOR_LOGIN` never appears in a launched AgentRun's secretEnv + # for any webhook-driven turn. Same secret/value integration-gateway and + # agent-orchestrator already share for this exact purpose. + secretName: e2e-integration-gateway-secrets identity: # The Go gateway resolves ITS OWN caller identity from a bearer token -- # entirely separate from agent-orchestrator's own `config.staticIdentities` diff --git a/e2e/specs/bridged-agent-workflow.e2e.ts b/e2e/specs/bridged-agent-workflow.e2e.ts index e729b03..77f66c0 100644 --- a/e2e/specs/bridged-agent-workflow.e2e.ts +++ b/e2e/specs/bridged-agent-workflow.e2e.ts @@ -96,13 +96,15 @@ const REPO = "e2e-bridged-repo"; const STUB_REPLY_MARKER = "stub-agent-reply"; /** - * The Temporal engine's Go gateway resolves every internal, tokenless call - * from agent-orchestrator to this subject (`gateway.identity.defaultSubject`, - * values-e2e.yaml) -- there is no per-end-user token on this hop regardless of - * which GitHub user triggered the webhook, so this is the subject a credential - * must be seeded under for the engine's own identity-link gate to resolve it. + * Cross-entry-point providers (claude, claude-remote) key by PRINCIPAL, not + * by `gateway.identity.defaultSubject` -- a webhook turn's signed sender + * assertion resolves `principal = github:` regardless of which + * shared subject the tokenless internal hop itself defaults to (see + * identity-keying.e2e.ts's CANONICAL, which this mirrors). This is the + * subject a credential must be seeded under for the engine's identity-link + * gate to resolve it. */ -const ENGINE_CALLER_SUBJECT = "client-integration-gateway"; +const CANONICAL = `github:${SENDER}`; describe("Temporal engine: a bridged pod agent actually runs as BridgedAgentWorkflow", () => { let secret: string; @@ -116,7 +118,7 @@ describe("Temporal engine: a bridged pod agent actually runs as BridgedAgentWork // claude-code-swe-agent's own identityProviders) -- see this suite's // credential-store.ts doc comment for why seeding is the only hermetic way // to get an authorized (not link-required) verdict. - await seedAllClaudeCredentials(ENGINE_CALLER_SUBJECT); + await seedAllClaudeCredentials(CANONICAL); }); afterAll(async () => { diff --git a/e2e/support/k8s.ts b/e2e/support/k8s.ts index 8e61924..1f97f1d 100644 --- a/e2e/support/k8s.ts +++ b/e2e/support/k8s.ts @@ -202,7 +202,7 @@ export async function agentRunsSince(since: Date): Promise<{ name: string; phase export async function jobEnvNames(agentRunName: string): Promise { const job = await kubectlJson<{ spec: { template: { spec: { containers: { env?: { name: string }[] }[] } } }; - }>(["get", "job", `agentrun-${agentRunName}`]); + }>(["get", "job", agentRunName]); return (job.spec.template.spec.containers[0]?.env ?? []).map((e) => e.name); } diff --git a/engines/temporal/internal/authz/authz.go b/engines/temporal/internal/authz/authz.go index 7255b58..9db54e9 100644 --- a/engines/temporal/internal/authz/authz.go +++ b/engines/temporal/internal/authz/authz.go @@ -274,7 +274,7 @@ func New(deps Deps) *Service { return &Service{deps: deps} } var providerLabel = map[string]string{ identitylink.ProviderGitHub: "GitHub", identitylink.ProviderClaude: "Claude", - identitylink.ProviderClaudeRemote: "Claude (Remote Control)", + identitylink.ProviderClaudeRemote: "Claude Remote Control", } func label(provider string) string { @@ -745,28 +745,43 @@ func linkPromptText(started identitylink.StartResult, label string) string { // composeLinkRequired states every outstanding link in one message. The batch // shape is the point: a caller authorizes once for everything the run needs // rather than discovering the next gap on the next trigger. +// +// Wording matches upstream's composeLinkRequiredMessage +// (apps/agent-orchestrator/src/agent/authorization-service.ts) exactly, +// including the "N accounts" count in the multi-link case — a caller-facing +// string, not just an implementation detail, and one this port had drifted +// from silently (no error, just different prose than upstream's). func composeLinkRequired(pending []pendingEntry, failedToStart []string) string { - var b strings.Builder + var parts []string + switch len(pending) { case 0: case 1: - fmt.Fprintf(&b, "To continue, please %s. This is a one-time step.", pending[0].linkText) + parts = append(parts, fmt.Sprintf( + "To continue, please %s. This is a one-time step -- send any message once you're done.", + pending[0].linkText)) default: - b.WriteString("To continue, please link the accounts this needs:") - for _, p := range pending { - fmt.Fprintf(&b, "\n- %s", p.linkText) + texts := make([]string, len(pending)) + for i, p := range pending { + texts[i] = p.linkText } - b.WriteString("\n\nThese are one-time steps.") + parts = append(parts, fmt.Sprintf( + "To continue, I need you to link %d accounts (one-time). Please %s. Send any message once you're done.", + len(pending), strings.Join(texts, ", and "))) } if len(failedToStart) > 0 { - if b.Len() > 0 { - b.WriteString("\n\n") + labels := strings.Join(failedToStart, " and ") + if len(parts) > 0 { + parts = append(parts, fmt.Sprintf( + "I also couldn't start the %s linking step just now -- try again in a moment and I'll retry that part.", labels)) + } else { + parts = append(parts, fmt.Sprintf( + "I couldn't start the one-time %s account-linking step just now. Please try again in a moment -- re-apply the label or send any message and I'll retry.", labels)) } - fmt.Fprintf(&b, "I also couldn't start the %s linking step just now — please try again shortly.", - strings.Join(failedToStart, " and ")) } - return b.String() + + return strings.Join(parts, " ") } func pendingKeys(pending []pendingEntry) []string { diff --git a/engines/temporal/internal/authz/authz_test.go b/engines/temporal/internal/authz/authz_test.go index 038e602..39a9b24 100644 --- a/engines/temporal/internal/authz/authz_test.go +++ b/engines/temporal/internal/authz/authz_test.go @@ -129,7 +129,7 @@ func TestEveryMissingProviderIsReportedTogether(t *testing.T) { require.NoError(t, err) require.Equal(t, authz.KindLinkRequired, verdict.Kind) require.Contains(t, verdict.Message, "GitHub") - require.Contains(t, verdict.Message, "Claude (Remote Control)") + require.Contains(t, verdict.Message, "Claude Remote Control") require.NotNil(t, verdict.Pending) } diff --git a/engines/temporal/internal/gateway/invoke.go b/engines/temporal/internal/gateway/invoke.go index fd7328a..59e457f 100644 --- a/engines/temporal/internal/gateway/invoke.go +++ b/engines/temporal/internal/gateway/invoke.go @@ -58,6 +58,27 @@ type invokeRequest struct { // workflow re-resolves whatever is named under the caller's own roles. ForcedSkillID string `json:"forcedSkillId,omitempty"` ForcedAgentID string `json:"forcedAgentId,omitempty"` + + // CallerTools / CallerToolRequired carry a chat turn's caller-supplied + // tools (ADR 0035) across this hop, already parsed, validated and ranked + // by agent-orchestrator's own handleChat before it ever calls + // TemporalEngine.invoke() -- so this is the resolved Descriptor shape, not + // the raw OpenAI `tools`/`tool_choice` request body callertools.Parse + // consumes. Re-parsing/re-ranking here would duplicate work already done + // (a second, redundant Qdrant rank against a fresh top-K) and there is no + // raw tool list left to re-parse by this point in agent-orchestrator's own + // pipeline. Omitting these silently drops every caller tool for any turn + // routed through /invoke, regardless of what the original chat request + // offered. + CallerTools []callertools.Descriptor `json:"callerTools,omitempty"` + CallerToolRequired bool `json:"callerToolRequired,omitempty"` + + // PriorCallerToolCalls are calls the client already executed and reported + // back (ADR 0035 resume), forwarded verbatim from agent-orchestrator's own + // `actionHistory` -- there is no raw message array at this hop for + // callertools.CollectPriorCalls to parse, unlike handleChat talking + // directly to a client. + PriorCallerToolCalls []callertools.PriorCall `json:"priorCallerToolCalls,omitempty"` } type invokeAccepted struct { @@ -93,6 +114,27 @@ const invokePollTimeout = 2 * time.Second // errEmptyRequest is the one shaping failure a caller can fix. var errEmptyRequest = errors.New(`body must be JSON: {"request": ""}`) +// resolveInvokeCaller prefers a trusted, signed caller identity +// (rbac.CallerIdentityHeader) over this gateway's own bearer-token +// resolution, when present and valid. +// +// Every /invoke caller today is agent-orchestrator, authenticating with ONE +// shared service token (or none). Bearer resolution alone therefore collapses +// every Open WebUI user, and every webhook sender, onto the SAME subject -- +// agent-orchestrator has ALREADY resolved the real per-request identity +// itself (OIDC/static/forwarded-user-JWT) before this hop, and this header is +// how it proves that to the gateway instead of the gateway re-deriving it +// from a token it was never given. +func (s *Server) resolveInvokeCaller(c *gin.Context) activities.Caller { + caller := resolveCaller(c, s.identity) + if subject, roles, perUser := rbac.VerifyCallerIdentityAssertion(s.senderAssertionSecret, c.GetHeader(rbac.CallerIdentityHeader), time.Now()); subject != "" { + caller.Subject = subject + caller.Roles = roles + caller.PerUser = perUser + } + return caller +} + // shapeInvokeTurn turns an /invoke body into the turn the workflow runs: // resolves who the adapter is vouching for, matches the event against the // route table, and renders the matched route's prompt. @@ -168,7 +210,7 @@ func (s *Server) handleInvoke(c *gin.Context) { c.GetHeader(rbac.SenderAssertionHeader), s.senderAssertionSecret, s.routes, - resolveCaller(c, s.identity), + s.resolveInvokeCaller(c), time.Now(), ) if err != nil { @@ -176,6 +218,17 @@ func (s *Server) handleInvoke(c *gin.Context) { return } + // Already resolved by the caller (agent-orchestrator's handleChat, ADR + // 0035) -- passed straight through, not re-parsed/re-ranked. See + // invokeRequest.CallerTools's doc comment for why. + if len(req.CallerTools) > 0 { + turn.CallerTools = req.CallerTools + turn.CallerToolRequired = req.CallerToolRequired + } + if len(req.PriorCallerToolCalls) > 0 { + turn.PriorCallerToolCalls = req.PriorCallerToolCalls + } + sessionID := strings.TrimSpace(req.SessionID) if sessionID == "" { sessionID = uuid.NewString() diff --git a/engines/temporal/internal/identitylink/identitylink.go b/engines/temporal/internal/identitylink/identitylink.go index 1e9386b..3045954 100644 --- a/engines/temporal/internal/identitylink/identitylink.go +++ b/engines/temporal/internal/identitylink/identitylink.go @@ -372,13 +372,19 @@ func (c *Client) Rekey(ctx context.Context, provider, fromSubject, toSubject str if mode != "" { body["mode"] = mode } + // The route's response is `{status: "moved"|"not-found"|"occupied"}` + // (integration-gateway's ClaudeAuthStore.rekey union), NOT a boolean + // `moved` field -- that field never existed on the wire, so a client + // reading it silently decoded false for every response including a real + // "moved", making adopt() believe every rekey failed and fall through to + // re-prompting the caller instead of ever moving their credential. var out struct { - Moved bool `json:"moved"` + Status string `json:"status"` } if _, err := c.do(ctx, http.MethodPost, "/claude-auth/api/rekey", body, &out); err != nil { return false, err } - return out.Moved, nil + return out.Status == "moved", nil } func (c *Client) WritebackGrant(ctx context.Context, provider, subject string, ttl time.Duration) (*WritebackGrant, error) { diff --git a/engines/temporal/internal/identitylink/rekey_test.go b/engines/temporal/internal/identitylink/rekey_test.go new file mode 100644 index 0000000..91d127b --- /dev/null +++ b/engines/temporal/internal/identitylink/rekey_test.go @@ -0,0 +1,69 @@ +package identitylink_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/controller-agent/temporal-engine/internal/identitylink" +) + +// The route's real response shape (integration-gateway's ClaudeAuthStore.rekey +// union: "moved" | "not-found" | "occupied"), not a boolean `moved` field — +// see Rekey's doc comment for the bug this pins: a client reading `moved` +// decoded false for every response, including a real "moved", so adopt() +// believed every rekey failed and always fell through to re-prompting the +// caller instead of ever moving their credential. +func newRekeyServer(t *testing.T, status string) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/claude-auth/api/rekey", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"status": status}) + })) +} + +func TestRekey_Moved(t *testing.T) { + server := newRekeyServer(t, "moved") + defer server.Close() + client := identitylink.New(identitylink.Options{BaseURL: server.URL}) + + moved, err := client.Rekey(t.Context(), identitylink.ProviderClaude, "openwebui:alice", "github:alice") + require.NoError(t, err) + require.True(t, moved) +} + +func TestRekey_NotFound(t *testing.T) { + server := newRekeyServer(t, "not-found") + defer server.Close() + client := identitylink.New(identitylink.Options{BaseURL: server.URL}) + + moved, err := client.Rekey(t.Context(), identitylink.ProviderClaude, "openwebui:alice", "github:alice") + require.NoError(t, err) + require.False(t, moved) +} + +func TestRekey_Occupied(t *testing.T) { + server := newRekeyServer(t, "occupied") + defer server.Close() + client := identitylink.New(identitylink.Options{BaseURL: server.URL}) + + moved, err := client.Rekey(t.Context(), identitylink.ProviderClaude, "openwebui:alice", "github:alice") + require.NoError(t, err) + require.False(t, moved) +} + +func TestRekey_NonClaudeProviderNeverCallsOut(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("github must never be rekeyed -- it produces the mapping, keying it by principal would be circular") + })) + defer server.Close() + client := identitylink.New(identitylink.Options{BaseURL: server.URL}) + + moved, err := client.Rekey(t.Context(), identitylink.ProviderGitHub, "openwebui:alice", "github:alice") + require.NoError(t, err) + require.False(t, moved) +} diff --git a/engines/temporal/internal/rbac/caller_identity_assertion.go b/engines/temporal/internal/rbac/caller_identity_assertion.go new file mode 100644 index 0000000..2f4575c --- /dev/null +++ b/engines/temporal/internal/rbac/caller_identity_assertion.go @@ -0,0 +1,98 @@ +package rbac + +import ( + "crypto/hmac" + "encoding/base64" + "encoding/json" + "time" +) + +// CallerIdentityHeader carries agent-orchestrator's signed claim about which +// per-request identity it already resolved for a chat/invoke turn -- the +// caller's OWN OIDC/static/forwarded-user-JWT resolution, NOT this gateway's +// bearer-token map. +// +// Every internal hop from agent-orchestrator otherwise authenticates with ONE +// shared service token (or none, degrading to WithDefaultIdentity), so every +// caller resolves to the SAME subject regardless of which human is actually +// chatting -- collapsing every Open WebUI user onto one identity, the same +// bug class ADR 0030 fixed for webhooks via SenderAssertionHeader. This is +// that fix generalized: a resolved SUBJECT (and its roles) rather than a +// GitHub login, signed for the same reason -- an unsigned field would let +// anything holding the gateway's token name an arbitrary subject. +const CallerIdentityHeader = "x-gateway-caller-identity" + +// callerIdentityPayload is the claim set. Field order matters for the same +// reason as assertionPayload's: it must match TypeScript's +// mintCallerIdentityAssertion field-insertion order for the signature to +// verify across implementations. +type callerIdentityPayload struct { + Subject string `json:"subject"` + Roles []string `json:"roles"` + // PerUser mirrors activities.Caller.PerUser (see its doc comment): true + // only when agent-orchestrator resolved Subject from a per-request signed + // identity, never from a shared static/bearer token. + PerUser bool `json:"perUser"` + Exp int64 `json:"exp"` // unix seconds +} + +// MintCallerIdentityAssertion produces `.` for a resolved +// subject/roles/perUser triple. Same HMAC scheme as MintSenderAssertion +// (reuses this package's signAssertion), deliberately kept as a separate +// payload/header rather than overloading assertionPayload: this asserts a +// resolved caller identity, not a GitHub login, and the two claims are +// verified independently by different call sites. +func MintCallerIdentityAssertion(secret, subject string, roles []string, perUser bool, ttl time.Duration, now time.Time) string { + payload := callerIdentityPayload{Subject: subject, Roles: roles, PerUser: perUser, Exp: now.Unix() + int64(ttl.Seconds())} + raw, err := json.Marshal(payload) + if err != nil { + return "" + } + payloadB64 := base64.RawURLEncoding.EncodeToString(raw) + return payloadB64 + "." + signAssertion(secret, payloadB64) +} + +// VerifyCallerIdentityAssertion returns the asserted subject/roles/perUser, +// or ("", nil, false) if the assertion is missing, malformed, expired, or not +// signed by secret. Fails closed and silently, same discipline as +// VerifySenderAssertion. +func VerifyCallerIdentityAssertion(secret, assertion string, now time.Time) (string, []string, bool) { + if secret == "" || assertion == "" { + return "", nil, false + } + + dot := -1 + for i := 0; i < len(assertion); i++ { + if assertion[i] == '.' { + if dot >= 0 { + return "", nil, false + } + dot = i + } + } + if dot <= 0 || dot == len(assertion)-1 { + return "", nil, false + } + payloadB64, signature := assertion[:dot], assertion[dot+1:] + + expected := signAssertion(secret, payloadB64) + if !hmac.Equal([]byte(expected), []byte(signature)) { + return "", nil, false + } + + raw, err := base64.RawURLEncoding.DecodeString(payloadB64) + if err != nil { + return "", nil, false + } + var payload callerIdentityPayload + if err := json.Unmarshal(raw, &payload); err != nil { + return "", nil, false + } + if payload.Subject == "" || payload.Exp <= 0 { + return "", nil, false + } + if payload.Exp*1000 <= now.UnixMilli() { + return "", nil, false + } + return payload.Subject, payload.Roles, payload.PerUser +} diff --git a/engines/temporal/internal/rbac/caller_identity_assertion_test.go b/engines/temporal/internal/rbac/caller_identity_assertion_test.go new file mode 100644 index 0000000..4e8513b --- /dev/null +++ b/engines/temporal/internal/rbac/caller_identity_assertion_test.go @@ -0,0 +1,64 @@ +package rbac_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/controller-agent/temporal-engine/internal/rbac" +) + +// Vector generated by agent-orchestrator's own mintCallerIdentityAssertion +// (apps/agent-orchestrator/src/rbac/caller-identity-assertion.ts) — same +// cross-implementation discipline as sender_assertion_test.go's tsVectors: an +// assertion minted by the TypeScript side must verify here. +func TestVerifyCallerIdentityAssertion_TypeScriptVector(t *testing.T) { + assertion := "eyJzdWJqZWN0Ijoib3BlbndlYnVpOmFsaWNlIiwicm9sZXMiOlsicmVhZGVyIiwid3JpdGVyIl0sInBlclVzZXIiOnRydWUsImV4cCI6MTcwMDAwMDMwMH0.2OvGNdQWfCb9P0Wge72LN_60Q2fPcYIpNHHXaTQm60Q" + now := time.UnixMilli(1700000000000 - 100_000) // before the vector's exp + + subject, roles, perUser := rbac.VerifyCallerIdentityAssertion("s3cr3t", assertion, now) + require.Equal(t, "openwebui:alice", subject) + require.Equal(t, []string{"reader", "writer"}, roles) + require.True(t, perUser) +} + +func TestCallerIdentityAssertion_RoundTrip(t *testing.T) { + now := time.Now() + assertion := rbac.MintCallerIdentityAssertion("s3cr3t", "openwebui:bob", []string{"reader"}, true, rbac.DefaultAssertionTTL, now) + + subject, roles, perUser := rbac.VerifyCallerIdentityAssertion("s3cr3t", assertion, now) + require.Equal(t, "openwebui:bob", subject) + require.Equal(t, []string{"reader"}, roles) + require.True(t, perUser) +} + +func TestCallerIdentityAssertion_RoundTrip_NotPerUser(t *testing.T) { + now := time.Now() + assertion := rbac.MintCallerIdentityAssertion("s3cr3t", "client-integration-gateway", []string{"reader", "writer"}, false, rbac.DefaultAssertionTTL, now) + + subject, roles, perUser := rbac.VerifyCallerIdentityAssertion("s3cr3t", assertion, now) + require.Equal(t, "client-integration-gateway", subject) + require.Equal(t, []string{"reader", "writer"}, roles) + require.False(t, perUser) +} + +func TestVerifyCallerIdentityAssertion_WrongSecret(t *testing.T) { + now := time.Now() + assertion := rbac.MintCallerIdentityAssertion("s3cr3t", "openwebui:bob", []string{"reader"}, true, rbac.DefaultAssertionTTL, now) + + subject, roles, perUser := rbac.VerifyCallerIdentityAssertion("wrong-secret", assertion, now) + require.Empty(t, subject) + require.Nil(t, roles) + require.False(t, perUser) +} + +func TestVerifyCallerIdentityAssertion_Expired(t *testing.T) { + now := time.Now() + assertion := rbac.MintCallerIdentityAssertion("s3cr3t", "openwebui:bob", []string{"reader"}, true, time.Second, now.Add(-time.Hour)) + + subject, roles, perUser := rbac.VerifyCallerIdentityAssertion("s3cr3t", assertion, now) + require.Empty(t, subject) + require.Nil(t, roles) + require.False(t, perUser) +}