docs(workflow-executor): document FOREST_EXECUTOR_ENCRYPTION_KEY

[hercemer42]

What

Documents FOREST_EXECUTOR_ENCRYPTION_KEY in the workflow-executor README — a new "OAuth-protected MCP connectors" section covering:

• what it encrypts (the OAuth credentials the executor stores, AES-256-GCM at rest)
• how to generate it (openssl rand -hex 32) and to keep it separate from FOREST_AUTH_SECRET
• using the same value across instances that share a database
• its lazy behaviour (while unset, credential storage returns 503 and those connectors stay unavailable; the executor still boots, with a startup warning)
• no managed rotation — changing it forces affected users to reconnect

It complements the one-line entry already in .env.example.

Why

PRD-626 (the docs half of PRD-367): operators deploying the executor with OAuth-protected MCP connectors need a README reference for this variable.

Base / merge order

Based on the PRD-692 branch (feature/prd-692-harden-executor-oauth-runtime-clear-idempotency-phase-on, #1724) — the top of the OAuth-MCP executor stack, where FOREST_EXECUTOR_ENCRYPTION_KEY and the code that reads it already live. Targeting that branch keeps this diff to just the README change. Intended to merge last, once PR1 (#1619), PR2 (#1665) and PRD-692 (#1724) are in.

Replaces docs-site PR ForestAdmin/docs#3 (now closed) — the executor's own README, next to the code and .env.example, is the right home for an operator-facing deployment variable (and "executor" isn't a concept on the public docs site).

Refs: PRD-626

🤖 Generated with Claude Code

Note

Add OAuth2 token refresh and MCP tool re-authentication to the workflow executor

• Introduces OAuthTokenService to acquire, cache, and refresh OAuth2 access tokens per user and MCP server, using a per-key mutex to serialize concurrent refreshes and proactively refreshing before expiry.
• RemoteToolFetcher now injects Bearer tokens into MCP configs for OAuth2-backed servers and retries once with a forced refresh on 401-like failures; persistent auth failures throw OAuthReauthRequiredError.
• McpStepExecutor intercepts auth errors during tool invocation, retries via a reloadWithFreshAuth callback, and converts a second failure to an awaiting-input outcome with awaitingInputReason: 'needs-oauth-reauth'.
• Adds a GET /list-mcp-tools HTTP endpoint that fetches available tools for the authenticated user's stored credential, returning HTTP 409 on re-auth required and 503 on missing encryption key.
• Adds SSRF protection via assertSafeTokenEndpoint, which rejects link-local, metadata, unspecified, and (in production) loopback token endpoint URLs.
• Documents FOREST_EXECUTOR_ENCRYPTION_KEY (AES-256-GCM) in README.md, covering generation, deployment, and rotation guidance.
• Risk: Runner and ExecutorHttpServer constructors now require additional OAuth service dependencies; existing instantiation without them will fail to compile.

📊 Macroscope summarized e3db719. 9 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

[linear-code]

PRD-367

PRD-626

[qltysh]

Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

Modified Files with Diff Coverage (26)

Rating	File	% Diff	Uncovered Line #s
	...ow-executor/src/stores/database-mcp-oauth-credentials-store.ts	100.0%
	packages/workflow-executor/src/runner.ts	100.0%
	packages/workflow-executor/src/executors/step-executor-factory.ts	100.0%
	packages/workflow-executor/src/oauth/refresh-grant.ts	100.0%
	packages/workflow-executor/src/build-workflow-executor.ts	100.0%
	packages/workflow-executor/src/oauth/token-service.ts	98.2%	153
	packages/workflow-executor/src/remote-tool-fetcher.ts	100.0%
	packages/workflow-executor/src/stores/in-memory-store.ts	100.0%
	packages/workflow-executor/src/http/executor-http-server.ts	95.5%	348
	packages/workflow-executor/src/oauth/token-endpoint-url.ts	100.0%
	packages/ai-proxy/src/mcp-auth-error.ts	92.3%	25-27
	packages/workflow-executor/src/adapters/server-ai-adapter.ts	100.0%
	packages/workflow-executor/src/types/validated/step-outcome.ts	100.0%
	packages/ai-proxy/src/mcp-client.ts	100.0%
	...w-executor/src/stores/in-memory-mcp-oauth-credentials-store.ts	100.0%
	packages/workflow-executor/src/adapters/ai-client-adapter.ts	100.0%
	packages/ai-proxy/src/ai-client.ts	100.0%
	packages/workflow-executor/src/errors.ts	100.0%
	packages/workflow-executor/src/oauth/keyed-mutex.ts	100.0%
	packages/workflow-executor/src/executors/mcp-step-executor.ts	100.0%
	...ow-executor/src/adapters/step-outcome-to-update-step-mapper.ts	100.0%
	...s/workflow-executor/src/adapters/always-error-ai-model-port.ts	100.0%
	packages/workflow-executor/src/defaults.ts	100.0%
	packages/workflow-executor/src/stores/database-store.ts	100.0%
	packages/ai-proxy/src/index.ts	100.0%
	packages/workflow-executor/src/http/mcp-oauth-credentials.ts	100.0%
	Total	98.6%

🤖 Increase coverage with AI coding...

In the `docs/prd-367-pr4-executor-encryption-key` branch, add test coverage for this new code:

- `packages/ai-proxy/src/mcp-auth-error.ts` -- Line 25-27
- `packages/workflow-executor/src/http/executor-http-server.ts` -- Line 348
- `packages/workflow-executor/src/oauth/token-service.ts` -- Line 153

🚦 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.

[Scra3]

Docs review (PRD-367 PR4). The section is accurate against the implementation; one small suggestion.

[Scra3]

suggestion: unlike FOREST_ENV_SECRET, the key's length/format is not validated by the code — worth noting operators must use the exact openssl rand -hex 32 output, since a weak value is accepted silently.

[qltysh]

12 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 9): create	5
qlty	Structure	Function with many parameters (count = 4): constructor	4
qlty	Structure	Function with high complexity (count = 12): invokeWithReauthRetry	3

[macroscopeapp]

🟡 Medium executors/mcp-step-executor.ts:94

doExecute deletes the entire persisted MCP execution when OAuthReauthRequiredError is caught. When the step is resumed, runStep() finds no pending data, so it falls back to selectTool() and re-runs the LLM tool selection. This discards the previously chosen (and possibly user-confirmed) tool call — in fully-automated mode the resumed step can execute a different tool or input, and in confirmation mode the user's confirmed McpToolCall is lost. Consider clearing only the idempotencyPhase marker (or setting it to a value that lets runStep() reuse the existing pendingData) instead of deleting the whole execution record.

Suggested change

	await this.context.runStore.deleteStepExecution(this.context.runId, this.context.stepIndex);
	await this.context.runStore.saveStepExecution(this.context.runId, {
	...existingExecution,
	type: 'mcp',
	stepIndex: this.context.stepIndex,
	idempotencyPhase: undefined,
	});

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/workflow-executor/src/executors/mcp-step-executor.ts around line 94:

`doExecute` deletes the entire persisted MCP execution when `OAuthReauthRequiredError` is caught. When the step is resumed, `runStep()` finds no pending data, so it falls back to `selectTool()` and re-runs the LLM tool selection. This discards the previously chosen (and possibly user-confirmed) tool call — in fully-automated mode the resumed step can execute a different tool or input, and in confirmation mode the user's confirmed `McpToolCall` is lost. Consider clearing only the `idempotencyPhase` marker (or setting it to a value that lets `runStep()` reuse the existing `pendingData`) instead of deleting the whole execution record.

[macroscopeapp]

🟡 Medium stores/database-mcp-oauth-credentials-store.ts:199

updateIfPresent overwrites every column from a stale snapshot, not just the rotated refresh_token_enc. If the user reconnects the same mcpServerId between the initial get() and this write-back, the new row still matches WHERE user_id = :userId AND mcp_server_id = :mcpServerId, so this statement overwrites the freshly re-authenticated client_id, client_secret_enc, token_endpoint, and scopes with old values, corrupting the new credential and breaking later refreshes. Consider scoping the update to only the rotated refresh token (e.g., SET refresh_token_enc = :refreshTokenEnc, updated_at = :now), or tracking the row id/version from the get() to avoid touching a re-created row.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/workflow-executor/src/stores/database-mcp-oauth-credentials-store.ts around line 199:

`updateIfPresent` overwrites every column from a stale snapshot, not just the rotated `refresh_token_enc`. If the user reconnects the same `mcpServerId` between the initial `get()` and this write-back, the new row still matches `WHERE user_id = :userId AND mcp_server_id = :mcpServerId`, so this statement overwrites the freshly re-authenticated `client_id`, `client_secret_enc`, `token_endpoint`, and `scopes` with old values, corrupting the new credential and breaking later refreshes. Consider scoping the update to only the rotated refresh token (e.g., `SET refresh_token_enc = :refreshTokenEnc, updated_at = :now`), or tracking the row id/version from the `get()` to avoid touching a re-created row.

[macroscopeapp]

🟡 Medium src/mcp-auth-error.ts:42

isMcpAuthError returns true for an error whose message matches /unauthorized/i even when a later link in the cause chain carries an explicit 403 status. For example, new Error('Unauthorized scope', { cause: Object.assign(new Error('forbidden'), { status: 403 }) }) is classified as refreshable auth, because .some() returns at the outer message match before the inner 403 is inspected. The comment states an explicit status is authoritative, but the implementation lets a message match short-circuit before any later status is checked. Consider scanning the full chain for an explicit non-refreshable status before falling back to message matching.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/ai-proxy/src/mcp-auth-error.ts around line 42:

`isMcpAuthError` returns `true` for an error whose message matches `/unauthorized/i` even when a later link in the cause chain carries an explicit `403` status. For example, `new Error('Unauthorized scope', { cause: Object.assign(new Error('forbidden'), { status: 403 }) })` is classified as refreshable auth, because `.some()` returns at the outer message match before the inner `403` is inspected. The comment states an explicit status is authoritative, but the implementation lets a message match short-circuit before any later status is checked. Consider scanning the full chain for an explicit non-refreshable status before falling back to message matching.

[macroscopeapp]

🟡 Medium oauth/token-service.ts:95

When runGrantWithRotationRetry throws OAuthReauthRequiredError, the cache entry for that key is left in place. A subsequent getAccessToken(..., { forceRefresh: false }) call returns the stale access token from readCache until its local expiry, so requests keep using a token whose refresh already failed instead of surfacing the re-auth requirement. Consider evicting the cache entry before throwing in refreshAndCache.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/workflow-executor/src/oauth/token-service.ts around line 95:

When `runGrantWithRotationRetry` throws `OAuthReauthRequiredError`, the cache entry for that key is left in place. A subsequent `getAccessToken(..., { forceRefresh: false })` call returns the stale access token from `readCache` until its local expiry, so requests keep using a token whose refresh already failed instead of surfacing the re-auth requirement. Consider evicting the cache entry before throwing in `refreshAndCache`.

[macroscopeapp]

🟡 Medium oauth/token-service.ts:87

readCache() returns undefined for expired entries but never deletes them from this.cache, and evict() is only called on explicit disconnect. Expired entries for (userId, mcpServerId) pairs accumulate in this.cache indefinitely, so process memory grows over the lifetime of the executor. Consider deleting the entry in readCache() when it is expired.

Suggested change

	private readCache(key: string): string | undefined {
	const entry = this.cache.get(key);
	if (!entry || entry.expiresAtMs === undefined) return undefined;
	if (this.now() >= entry.expiresAtMs - this.expirySkewMs) return undefined;
	return entry.accessToken;
	}
	private readCache(key: string): string | undefined {
	const entry = this.cache.get(key);
	if (!entry || entry.expiresAtMs === undefined) return undefined;
	if (this.now() >= entry.expiresAtMs - this.expirySkewMs) {
	this.cache.delete(key);
	return undefined;
	}
	return entry.accessToken;
	}

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/workflow-executor/src/oauth/token-service.ts around lines 87-93:

`readCache()` returns `undefined` for expired entries but never deletes them from `this.cache`, and `evict()` is only called on explicit disconnect. Expired entries for `(userId, mcpServerId)` pairs accumulate in `this.cache` indefinitely, so process memory grows over the lifetime of the executor. Consider deleting the entry in `readCache()` when it is expired.

[macroscopeapp]

🟡 Medium

agent-nodejs/packages/workflow-executor/src/build-workflow-executor.ts

Line 198 in e3db719

async start() {

createWorkflowExecutor.start() calls runner.start() before server.start(), so the polling loop can invoke DatabaseMcpOAuthCredentialsStore.get() — which issues SELECT against ai_mcp_oauth_credentials — before ExecutorHttpServer.start() runs mcpOAuthCredentialsStore.init() to create the table. On startup with a pending OAuth-backed step, the first poll cycle fails with a missing-table error instead of executing the run. Consider awaiting the store migration before runner.start(), or calling init() explicitly in buildDatabaseExecutor prior to starting the runner.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/workflow-executor/src/build-workflow-executor.ts around line 198:

`createWorkflowExecutor.start()` calls `runner.start()` before `server.start()`, so the polling loop can invoke `DatabaseMcpOAuthCredentialsStore.get()` — which issues `SELECT` against `ai_mcp_oauth_credentials` — before `ExecutorHttpServer.start()` runs `mcpOAuthCredentialsStore.init()` to create the table. On startup with a pending OAuth-backed step, the first poll cycle fails with a missing-table error instead of executing the run. Consider awaiting the store migration before `runner.start()`, or calling `init()` explicitly in `buildDatabaseExecutor` prior to starting the runner.

[macroscopeapp]

🟡 Medium oauth/refresh-grant.ts:50

buildRequest encodes clientId and clientSecret with encodeURIComponent before building the Basic auth header, which encodes spaces as %20. RFC 6749 requires the application/x-www-form-urlencoded algorithm for Basic auth credentials, which encodes spaces as +. Any client ID or secret containing a space sends malformed credentials and the refresh grant fails. Consider using URLSearchParams to encode each component instead.

Suggested change

	const credentials = `${encodeURIComponent(clientId ?? '')}:${encodeURIComponent(
	clientSecret,
	)}`;
	const credentials = `${new URLSearchParams('', { clientId: clientId ?? '' }).toString().slice('clientId='.length)}:${new URLSearchParams('', { clientSecret }).toString().slice('clientSecret='.length)}`;

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/workflow-executor/src/oauth/refresh-grant.ts around lines 50-52:

`buildRequest` encodes `clientId` and `clientSecret` with `encodeURIComponent` before building the Basic auth header, which encodes spaces as `%20`. RFC 6749 requires the `application/x-www-form-urlencoded` algorithm for Basic auth credentials, which encodes spaces as `+`. Any client ID or secret containing a space sends malformed credentials and the refresh grant fails. Consider using `URLSearchParams` to encode each component instead.