Conversation
3 new issues
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (5)
🛟 Help
|
Tonours
left a comment
Tonours
left a comment
There was a problem hiding this comment.
Spec (PRD-1372): conforms to the five acceptance criteria. The ticket asked to reuse agent-bff's parseApiKey, and the PR copies the pattern instead, with the move tracked as PRD-1409.
| } | ||
|
|
||
| if (!response.ok) { | ||
| const body = (await response.json().catch(() => ({}))) as SaasErrorBody; |
There was a problem hiding this comment.
🟠 High gateway-api-key/index.ts:85
A non-OK response with a valid JSON body of null throws a raw TypeError at body.errors, so it bypasses GatewayApiKeyResolveError and prevents the caller's refusal mapping. Normalize null to an empty object before reading errors.
| const body = (await response.json().catch(() => ({}))) as SaasErrorBody; | |
| const body = ((await response.json().catch(() => ({}))) ?? {}) as SaasErrorBody; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forestadmin-client/src/gateway-api-key/index.ts around line 85:
A non-OK response with a valid JSON body of `null` throws a raw `TypeError` at `body.errors`, so it bypasses `GatewayApiKeyResolveError` and prevents the caller's refusal mapping. Normalize `null` to an empty object before reading `errors`.
Tonours
left a comment
Tonours
left a comment
There was a problem hiding this comment.
Approved after /verify-fixes run 1944-20260930161056-Tonours-verify: 1 finding of 1944-20260930143229-Tonours closed, none reopened.
An fgw_ or fbff_ bearer is resolved on the Forest server with service mcp, cached, and turned into a short-lived agent token. The resolve client lives in forestadmin-client so mcp-server imports nothing from agent-bff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…laims shape Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ef0f579 to
5cd9fb1
Compare
Tonours
left a comment
Tonours
left a comment
There was a problem hiding this comment.
Spec (PRD-1372): conforms. The scope line asking to reuse agent-bff's parseApiKey is superseded by the 2026-09-30 decision recorded in PRD-1409.
| export { default as GatewayApiKeyResolveError } from './resolve-error'; | ||
| export type { GatewayApiKeyResolveErrorParams } from './resolve-error'; | ||
|
|
||
| const API_KEY_PATTERN = /^(?:fgw|fbff)_([0-9a-f]{16})_([0-9a-f]{64})$/; |
There was a problem hiding this comment.
This diff meets the criteria for a security review, so please run /security-review locally before merging.
Triggers
packages/forestadmin-client/src/gateway-api-key/index.ts:6parses a bearer credential received at the MCP trust boundary.packages/forestadmin-client/src/gateway-api-key/index.ts:72sends the environment secret to the resolve endpoint.packages/mcp-server/src/forest-oauth-provider.ts:569routes matching bearer tokens past JWT verification to a new auth path.packages/mcp-server/src/gateway-api-key-authenticator.ts:80caches authentication decisions keyed on a credential hash.packages/mcp-server/src/gateway-api-key-authenticator.ts:122signs an agent JWT with the agent auth secret.
There was a problem hiding this comment.
I ran /security-review again on HEAD 5cd9fb1: no finding at or above 0.8 confidence. It was a code read, with no exploit attempted.
Paths checked:
| Path | Why it is safe |
|---|---|
Auth path selection (forest-oauth-provider.ts:568-570) |
The key regex is anchored, has no m flag and accepts only lowercase hex (gateway-api-key/index.ts:6). A token cannot be both a valid JWT and a key. A key always goes through the server resolve. |
Cache (gateway-api-key-authenticator.ts:76-83,161-170) |
The cache key is sha256(keyId:secret), so a known keyId with a wrong secret never hits a cached identity. Only 401/403 are cached negatively; a 5xx cannot poison a valid key. Revocation can lag by up to 60 s, as in agent-bff (resolve-cache.ts:53-54). |
Agent JWT claims (gateway-api-key-authenticator.ts:119-141) |
Every claim comes from the resolve response, which is authenticated by the env secret and checked for shape (index.ts:118-155). Signing is the same as agent-bff since #1948: toAgentTokenClaims, HS256, 5 min. The agent still applies its own permissions (query-string.ts:194-200). Scopes match the OAuth default. |
Agent JWT replayed on /mcp |
The token is only sent to the environment's agent (agent-caller.ts:30-56), never to the client. If replayed anyway, it has no serverToken and no scopes, so DecodedAccessTokenSchema refuses it. |
| Agent JWT used as an upload handle | Refused: handles.ts:47-48 requires type: 'mcp-upload', and a handle is bound to its uploader. |
Secrets in logs (resolve-error.ts:10-24, forest-oauth-provider.ts:640-651) |
The key secret and env secret appear only in the request body and headers. The cause messages logged are: "fetch failed" plus the syscall and host:port, the timeout message, an invalid URL showing only forestServerUrl, a JSON parse error on the response, or a fixed string. None of them contain the request. saasAccessToken is never logged. |
| Errors returned to the client | OAuth errors carry fixed messages, and the cause never goes into the response body. Any other error becomes a generic 500. |
saasAccessToken |
Used like serverToken on the OAuth path (activity logs, Forest calls), and never sent to the client. |
🤖 Generated with Claude Code
This branch has not been deployed
2 participants
An autonomous system can call the Gateway MCP with a service account credential (
Authorization: Bearer fgw_…orfbff_…). It needs no OAuth flow and no human.Fixes PRD-1372
What
forestadmin-client/src/gateway-api-key/parseGatewayApiKey,GatewayApiKeyClient(resolve with aservice),GatewayApiKeyResolveErrormcp-server/src/gateway-api-key-authenticator.tsservice: 'mcp', cache, error mapping, mint a 5-minute agent JWTmcp-server/src/forest-oauth-provider.tsverifyAccessTokentries the credential first. JWTs never match the key pattern, so the OAuth path is unchangedmcp-serverREADME, CLAUDE.mdBehaviour
AuthInfo:tokenis the minted agent JWT.agent-callerforwards it unchanged, and nothing else readsauthInfo.token.clientId: service-account-key:<keyId>.expiresAtis the expiry of the agent JWT.mcp:read mcp:write mcp:action, the OAuth default. Real rights come from the service account's Role and Teams.forestServerTokenis the resolve'ssaasAccessToken. A resolve without one is refused.Refusals:
InvalidTokenError, 401plan_feature_missingInsufficientScopeError, 403, "The project's plan does not include the Gateway MCP"InsufficientScopeError, 403ServerError, 500A plan or identity refusal is a 403, never a 401: a 401 would send an MCP client into the OAuth flow.
Cache: success for 60 s, 401/403 for 10 s, 10,000 entries. This is the same grace window as agent-bff.
Why this shape
forestadmin-client, not agent-bff: the Gateway switch requires mcp-server to import nothing from agent-bff. Both packages already depend onforestadmin-client. Moving agent-bff onto it is PRD-1409.GatewayApiKeyClientcallsfetch, notServerUtils:ServerUtilsdropsmeta.codeandRetry-After, andplan_feature_missingmust be told apart.allowedOriginsis neither read nor required: it is on its way out (PRD-1386).Rollout
The server maps
mcpto no plan feature until PRD-1371 shipsgatewayMcp. Until then every credential on the MCP gets403 plan_feature_missing. Merging is safe; the dev end-to-end check waits for PRD-1371.Notes for reviewers
saasAccessTokenwithsessionType: bff. Billing still counts MCP reads: the activity log source comes fromForest-Application-Source: MCP.toAgentTokenClaims(fix: sign agent tokens in the shape Ruby agents read, and read both in the Node agent #1947), like the OAuth path and the workflow executor:tagsis a{key, value}array andrendering_ida string, the shape Ruby agents read. The Node agent turns the array back into a record. agent-bff'sissueAgentTokenstill signs a record.How to test
Definition of Done
General
Security
🤖 Generated with Claude Code
Note
Add service-account (
fgw_/fbff_) gateway credential support to MCP server authGatewayApiKeyClientinforestadmin-clientthat parsesfgw_/fbff_credentials and resolves them against the Forest server for themcpserviceForestOAuthProvider.verifyAccessTokentries gateway-key parsing before JWT verification; matching credentials resolve intoAuthInfowith a five-minute HS256 agent token andmcp:read/mcp:write/mcp:actionscopesInvalidTokenError, 403 →InsufficientScopeError(plan-feature or environment-access), others →ServerErrorMacroscope summarized 5cd9fb1.