feat(agent-bff): mint an agent token from the OAuth principal

[Tonours]

fixes PRD-866

Problem

OAuth bff_access tokens authenticate successfully, but Mode 1 requests fail on agent routes because no downstream agent token is created.

Fix

• Mint an agent token from the validated OAuth principal.
• Preserve the caller identity, including role, permissions, tags, and numeric renderingId.
• Map claims explicitly and exclude session-only type and sid claims.
• Fail closed when the principal has no usable rendering.
• Add regression coverage for OAuth routing, token claims, expiry, replay, and Mode 2 compatibility.

Scope and safety

• Changes are limited to packages/agent-bff.
• No SaaS round-trip is added.
• Mode 2 API-key authentication is unchanged.
• Session and CORS behavior outside the agent chain is unchanged.

How to test

yarn workspace @forestadmin/agent-bff test
yarn workspace @forestadmin/agent-bff build
yarn workspace @forestadmin/agent-bff lint

Definition of Done

General

• Write an explicit title for the Pull Request, following Conventional Commits specification
• Test manually the implemented changes
• Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

• Consider the security impact of the changes made

[linear-code]

PRD-866

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (2)

Rating	File	% Diff	Uncovered Line #s
	packages/agent-bff/src/api-key/agent-token.ts	100.0%
	packages/agent-bff/src/auth/auth-mode-middleware.ts	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[ShohanRahman]

Only rendering_id is validated at the trust boundary; other principal fields are silently coerced — src/api-key/agent-token.ts:88-96 + src/oauth/bff-token.ts:16-28 (raised independently by type-design + silent-failure hunter)
verifyBffAccess checks only type and exp, then does return decoded as BffAccessTokenPayload (auth-mode-middleware.ts:50). issueAgentTokenFromPrincipal then hard-validates rendering_id but passes id, email, team, permission_level straight through, and coerces first_name/last_name → '' and tags → {}. A structurally-broken but validly-signed token (schema drift, a bug in issueBffAccessToken, a future field rename) mints a "valid-looking" agent token with undefined/empty identity claims instead of failing loudly. Not an attack vector (same authSecret signs both, so forgery isn't possible), but a real robustness gap — downstream agents would trust a blank email/permissionLevel.
Fix: validate presence of the safety-critical fields (id, email, permission_level) at the boundary alongside rendering_id, throwing unauthorized rather than emitting empty/undefined claims. If blank names are a legitimate case, keep that default but make it deliberate.

2. BffAccessTokenPayload type is dishonest about nullability — src/oauth/bff-token.ts:21-22
   first_name: string / last_name: string are declared non-nullable, but the signer defends with ?? '' and the tests must write undefined as unknown as string to model a real state — a tell that the type lies. Sibling types (AgentCallerClaims, ApiKeyIdentityUser) correctly use string | null. Narrow these to string | null (matching role?), which removes the test casts and makes the ?? '' type-checked rather than silent defense.

3. rendering_id string handling has redundant coercion — src/api-key/agent-token.ts:82,93 (simplifier + type-design + silent-failure overlap)
   rendering_id is typed string, so POSITIVE_INTEGER.test(String(principal.rendering_id)) wraps a value already typed as a string. Either drop the String(...) wrapper, or (better, given it's really an unknown JWT claim) add an explicit typeof === 'string' check so a type-contract violation is distinguishable from a plain bad value. Longer term the field is number everywhere else in the domain — a branded/validated-once-at-boundary representation would remove the three-way String/regex/Number juggling.

[Tonours]

Thanks for digging in. I checked the three points against the code, here's where I land.

1. Only rendering_id is validated at the trust boundary — accurate diagnosis, but I'm not adding the checks.

You're right that verifyBffAccess does return decoded as BffAccessTokenPayload after checking only type and exp, and that issueAgentTokenFromPrincipal hard-validates rendering_id only.

But the only producer of a bff_access token is issueBffAccessToken in this same process, building a fully typed BffAccessTokenPayload from UserInfo — and every field of UserInfo is required (forestadmin-client/src/auth/types.ts:1-11). Same secret, same signer, same package. Validating id/email/permission_level at the boundary means defending against our own signer, which a test would catch first anyway.

rendering_id is validated for a different reason: it's the one field that round-trips String(renderingId) on the way out and Number(...) on the way back in. Without the guard, a malformed value lands as NaN in Caller.renderingId. That's a conversion guard, not a trust-boundary check — the two got conflated here.

This becomes a real concern the day a second bff_access issuer shows up. Not today.

2. BffAccessTokenPayload type is dishonest about nullability — not taking this one.

first_name is fed from user.firstName where user: UserInfo, and UserInfo.firstName is string, non-nullable. So the type is honest about its actual source. Widening it to string | null would make it lie about what issueBffAccessToken really produces.

The ?? '' in the shared signer isn't defending this path. It's there for ApiKeyIdentityUser.firstName, which genuinely is string | null. The signer is shared by both paths, so AgentCallerClaims.firstName: string | null is the correct supertype — those two types being different is the point, not drift.

On the undefined as unknown as string cast: it models a state the types say is impossible. That's a defensive test, not a tell that the type lies. And widening to string | null wouldn't remove it — the test writes undefined, not null.

3. Redundant String(...) coercion — accurate, but neither fix improves it.

rendering_id is typed string, so String(...) is a no-op per the types. Correct.

At runtime though, the value comes out of a decoded JWT — JSON.parse, not TypeScript. Dropping the wrapper leaves the regex coercing implicitly instead of explicitly. Adding typeof === 'string' rejects a rendering_id: 42 that String(42) would have handled fine — that's a regression on a legitimately number-typed token, traded for a stricter error message.

Your underlying point is the real one: the field is number everywhere else in the domain, and the String/regex/Number juggling exists only because Ruby/Python agents expect a string in the JWT. Worth a note, not a refactor in this PR.

What I did change: the no-store test name. It claimed the response "carries a minted credential" — it doesn't. The token lives in ctx.state.agentToken and is consumed by data-routes-middleware.ts / action-routes-middleware.ts into the outgoing agent call. Only the test handler echoes it. The old name made no-store look like the anti-leak control for the credential, which it isn't — it's just symmetry with the api-key branch. Renamed accordingly.

One thing worth flagging while we're here, pre-existing and outside this diff: the same FOREST_AUTH_SECRET signs both bff_access and the agent token, and the agent only verifies the signature. A stolen bff_access is already a signature-valid agent credential. Decided on 04/08, to be closed by a separate BFF_SESSION_SECRET. This PR doesn't make it worse — the minted token (5m, no type, no sid) is a strict subset of the bff_access it derives from. Should come back up before PRD-679.