feat(core): export A2A verification + owner-context seeding, add actionRouteAuth adapter for action routes#2047
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Visual recap — skippedThe visual recap job did not run for this pull request. This is informational only and does not block the PR. Recap skipped for |
|
Thanks for the thorough review — all three findings addressed in 875afc5:
+5 tests covering the aud matrix (present/absent × derivable/not), the org fallback, and null-vs-throw; touched suites 45/45 green, 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🔴
Review Details
This incremental pass reviewed the latest follow-up commits on PR #2047. The update fixes the previously reported ambient-session org leak: adapter-resolved callers now bypass resolveOrgId(event) entirely, the new ActionRouteResolvedCaller type makes the org source explicit, and the added tests cover hard-reject semantics plus cookie-vs-token precedence. I also reran the targeted core suites for the touched auth code; they pass locally.
Risk assessment: High. This PR still changes shared authentication and org-scoping behavior in @agent-native/core, so subtle mismatches between the exported A2A verifier and the new action-route adapter can become cross-org data-isolation bugs.
Key finding:
- 🔴 HIGH the direct action-route path still drops the token’s verified
org_domainunless the adapter manually converts it to anorgId, so multi-org A2A callers can execute under their default/active membership instead of the org the token was minted for.
The overall direction is strong and the earlier session-org override issue is fixed, but this remaining A2A org-resolution gap is still blocking for the new direct HTTP auth flow.
🧪 Browser testing: Skipped — PR only modifies core server/auth/test files, no UI impact.
| // original resolveOrgId-only behavior. | ||
| let orgId: string | undefined; | ||
| if (resolvedCaller) { | ||
| orgId = normalizeOrgId(resolvedCaller.orgId); |
There was a problem hiding this comment.
🔴 Direct A2A action auth drops the token’s verified org
For adapter-resolved callers without an explicit orgId, this path falls back to resolveOrgIdForEmail(owner), which picks the user’s active/default membership and ignores the JWT’s verified org_domain. A multi-org user can therefore mint a token from org B and still execute /_agent-native/actions/* under org A unless every adapter re-implements org_domain -> orgId; this route should mirror the existing A2A handler and resolve the verified org claim before falling back to email membership.
Additional Info
Found by 1/2 current review agents; confirmed against `verifyA2AToken()` returning `{ email, orgDomain }`, `signA2AToken()` embedding `org_domain`, and `packages/core/src/a2a/handlers.ts:317-330`, which already resolves verified org_domain before `resolveOrgIdForEmail`.
Code reviewFound 2 issues:
agent-native/packages/core/src/a2a/server.ts Lines 57 to 66 in f9c4ae4 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
|
thanks for the PR @ptahdunbar! couple things to look at above, then happy to get this in |
…onRouteAuth adapter Workspaces that accept A2A-authenticated callers on the direct HTTP action routes currently have to reach into core internals — hardcoding the private __agentNativeOwnerContext key string and reimplementing HS256 verification (losing org-level fallback secrets). All additive: - export verifyA2AToken + A2ATokenPayload from @agent-native/core/a2a (exact existing semantics incl. org-domain fallback secrets; event param now optional) - export AGENT_RUN_OWNER_CONTEXT_KEY, seedAgentRunOwnerContext, and AgentRunOwnerContext from the server barrel - new actionRouteAuth.resolveCaller adapter on mountActionRoutes / createAgentChatPlugin: runs before the getSession chain, seeds the resolved caller so nested agent runs observe it; null/throw defers to existing auth 9 new tests (verifyA2AToken semantics, adapter precedence/fallthrough); changeset included (minor).
…fallback for adapter callers, adapter throw = 401 - verifyA2AToken: a token carrying aud is rejected when the receiver cannot derive its own expected audience (previously verified signature-only — a correctly-signed token minted for another service was accepted when APP_URL/URL were unset). Tokens without aud keep existing behavior - action route adapter path: adapter-resolved callers now get the same owner-based org fallback as resolveAgentRunOrgId, so A2A-authenticated action calls carry orgId instead of writing org-scoped rows with NULL - ActionRouteAuthAdapter contract: resolveCaller returning null defers to the normal session chain; THROWING now hard-rejects 401 (an expired/forged bearer must not fall through to a same-origin session cookie); docs updated - +5 tests covering the aud matrix, org fallback, and null-vs-throw semantics
…edential A request can carry both a valid A2A bearer and an unrelated same-origin browser cookie. The adapter path previously resolved org via the app's session-backed resolveOrgId first, so the token caller's actions could execute under the cookie user's org. Adapter-resolved callers now take their org only from the credential: the adapter-returned orgId (ActionRouteResolvedCaller) or the owner-email membership lookup — never ambient session/org state.
bf60bf2 to
9104b61
Compare
There was a problem hiding this comment.
Builder reviewed your changes — no new findings
Review Details
This incremental review checked the latest PR #2047 state after the follow-up auth/org-scoping changes. I re-read the current diff for the shared @agent-native/core auth surfaces, with focus on the exported A2A verifier, actionRouteAuth adapter semantics, adapter-resolved org scoping, and owner-context seeding. I also reran the targeted suites that cover the touched paths: src/server/action-routes.spec.ts and src/a2a/server.spec.ts; both passed locally (48/48 tests).
Risk assessment: High. The PR is still modifying shared authentication and org-resolution behavior for direct HTTP action routes, so even small mismatches can become cross-org isolation issues.
For this incremental pass, I did not confirm any new issues beyond the already-open review comment that remains visible on the PR. That existing comment about direct A2A action auth dropping the token’s verified org still appears applicable based on the current code path, so I am not reposting it here.
The implementation continues to look materially improved versus earlier revisions (401 hard-reject behavior, cookie-org isolation tests, exported adapter types), but the PR should remain blocked until the existing open org-resolution concern is addressed.
🧪 Browser testing: Skipped — PR only modifies core server/auth/test files, no UI impact.
|
Both addressed:
|
|
thanks so much @ptahdunbar! great contribution |
Problem
Apps that want to accept A2A-authenticated callers on the direct HTTP action routes (
/_agent-native/actions/*) currently have to reach into core internals:verifyA2ATokenis internal, so workspaces re-implement HS256 verification themselves — and a hand-rolled version silently loses the org-level fallback-secret support the real implementation has.__agentNativeOwnerContextcontext-key string from a Nitrorequesthook, which drifts silently if core ever renames it.Changes (all additive — no behavior change for existing callers)
verifyA2AToken+A2ATokenPayloadfrom@agent-native/core/a2a. Exact existing semantics preserved (aud/iss/exp checks, org-domain fallback secrets); theeventparameter is now optional for callers outside a request context.AGENT_RUN_OWNER_CONTEXT_KEY,seedAgentRunOwnerContext, andAgentRunOwnerContextfrom the server barrel, so seeding is a typed API instead of a mirrored string constant.actionRouteAuthoption onmountActionRoutes/createAgentChatPlugin: an adapter whoseresolveCaller(event)runs before the cookie/bearergetSessionchain. Returning a caller seeds it viaseedAgentRunOwnerContext(so nested agent runs observe it); returningnull(or throwing) defers to the existing auth chain unchanged. This lets an app accept e.g. A2A JWTs on action routes declaratively:Tests
9 new tests:
verifyA2ATokenexport semantics (valid/invalid/expired/org-fallback) and adapter precedence/fallthrough on the action route (resolved caller used + seeded; null defers togetSession; throw defers safely). Touched suites: 40/40 pass;tsc --noEmitclean. Changeset included (minor).