mirror of
https://github.com/firecrawl/firecrawl-mcp-server.git
synced 2026-09-19 01:44:16 +08:00
fix(auth): let Core authenticate API keys instead of introspecting them
An API key is already the credential Core authenticates on every call. Resolving it through the authorization server returned the same key back, so the round trip added no authorization decision Core does not already make, while making every request depend on a third service. OAuth access tokens are unchanged: Core cannot resolve one on its own, so those still introspect and still fail closed. Forward the key and let Core be the authority. A rejection is no longer discovered at connect time, so translate a 401 from Core into the same CREDENTIAL_INVALID recovery the agent already knows how to act on. Tools reach Core two ways, the SDK helpers and the raw HTTP layer that search uses to keep its response envelope, and both carry the upstream status. Only 401 is matched. A 403 can mean an entitlement the key legitimately lacks, which is not a verdict on the credential. This gives up the credential_purpose check, which was defence in depth rather than a boundary since Core validates the key itself, and team metadata on API-key sessions, which fed action telemetry only.
This commit is contained in:
+58
-16
@@ -530,6 +530,16 @@ async function resolveCredentialFromHeaders(
|
||||
return { invalid: true };
|
||||
}
|
||||
|
||||
// An API key is already the credential Core authenticates, so introspection
|
||||
// resolved it to itself. That round trip bought no authorization decision
|
||||
// Core does not make on every call, while putting each request behind a third
|
||||
// service. Forward it and let Core be the authority; a rejection comes back as
|
||||
// the CREDENTIAL_INVALID recovery at the tool boundary. OAuth access tokens
|
||||
// still require introspection, because Core cannot resolve one on its own.
|
||||
if (isFirecrawlApiKey(token)) {
|
||||
return { credential: token, source: 'api-key' };
|
||||
}
|
||||
|
||||
let data = await introspectToken(token, profile.resourceUrl);
|
||||
if (
|
||||
isFirecrawlOAuthAccessToken(token) &&
|
||||
@@ -539,20 +549,7 @@ async function resolveCredentialFromHeaders(
|
||||
data = await introspectToken(token, DEFAULT_MCP_RESOURCE_URL);
|
||||
}
|
||||
if (!data.active || !data.api_key) {
|
||||
if (isFirecrawlOAuthAccessToken(token)) {
|
||||
throw new InvalidOAuthCredentialError();
|
||||
}
|
||||
return { invalid: true };
|
||||
}
|
||||
|
||||
if (isFirecrawlApiKey(token)) {
|
||||
return data.credential_purpose === 'general'
|
||||
? {
|
||||
credential: data.api_key,
|
||||
source: 'api-key',
|
||||
metadata: credentialMetadata(data),
|
||||
}
|
||||
: { invalid: true };
|
||||
throw new InvalidOAuthCredentialError();
|
||||
}
|
||||
const expectedAudience =
|
||||
profile.acceptLegacyAudience &&
|
||||
@@ -1127,6 +1124,46 @@ function invalidOAuthRecoveryPayload(): Record<string, unknown> & { message: str
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Core rejected the credential this session forwarded. Tools reach Core two
|
||||
* ways: the SDK helpers raise FirecrawlSdkError, while firecrawl_search posts
|
||||
* through the SDK's axios layer to keep the full response envelope. Both carry
|
||||
* the upstream status. Only 401 is matched, because that is a verdict on the
|
||||
* credential; a 403 can mean an entitlement the key legitimately lacks.
|
||||
*/
|
||||
function isCoreCredentialRejection(error: unknown): boolean {
|
||||
if (!(error instanceof Error)) return false;
|
||||
const candidate = error as {
|
||||
isAxiosError?: unknown;
|
||||
name?: string;
|
||||
response?: { status?: unknown };
|
||||
status?: unknown;
|
||||
};
|
||||
if (candidate.name !== 'FirecrawlSdkError' && candidate.isAxiosError !== true) {
|
||||
return false;
|
||||
}
|
||||
return candidate.status === 401 || candidate.response?.status === 401;
|
||||
}
|
||||
|
||||
/**
|
||||
* API keys reach Core unresolved, so an invalid one is discovered when a tool
|
||||
* runs rather than at connect time. Translate that rejection into the same
|
||||
* CREDENTIAL_INVALID recovery the agent already knows how to act on, so the
|
||||
* guidance does not depend on having introspected the key first.
|
||||
*/
|
||||
async function runWithCredentialRecovery<T>(
|
||||
run: () => T | Promise<T>,
|
||||
requestId: string
|
||||
): Promise<T> {
|
||||
try {
|
||||
return await run();
|
||||
} catch (error) {
|
||||
if (!isCoreCredentialRejection(error)) throw error;
|
||||
const payload = recoveryPayload('CREDENTIAL_INVALID', requestId);
|
||||
throw new UserError(String(payload.message), payload);
|
||||
}
|
||||
}
|
||||
|
||||
function recoveryPayload(
|
||||
code: string,
|
||||
requestId: string = randomUUID(),
|
||||
@@ -1333,11 +1370,16 @@ function guardHostedTool(
|
||||
if (logActions) emitActionLog(tool.name, 'error', invocationSession, new UserError(String(payload.message), payload), requestId, code);
|
||||
throw new UserError(String(payload.message), payload);
|
||||
}
|
||||
if (!logActions) return execute(args, invocationContext);
|
||||
const runTool = () =>
|
||||
runWithCredentialRecovery(
|
||||
() => execute(args, invocationContext),
|
||||
requestId
|
||||
);
|
||||
if (!logActions) return runTool();
|
||||
|
||||
emitActionLog(tool.name, 'started', invocationSession, undefined, requestId);
|
||||
try {
|
||||
const result = await execute(args, invocationContext);
|
||||
const result = await runTool();
|
||||
emitActionLog(tool.name, 'success', invocationSession, undefined, requestId);
|
||||
return result;
|
||||
} catch (error) {
|
||||
|
||||
@@ -151,6 +151,14 @@ async function startFakeBackend(options = {}) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Core is the authority on a forwarded API key, so the suite's invalid keys
|
||||
// are rejected here rather than at introspection.
|
||||
if ((req.headers.authorization ?? '').includes('invalid')) {
|
||||
res.writeHead(401, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify({ error: 'Unauthorized: Invalid token', success: false }));
|
||||
return;
|
||||
}
|
||||
|
||||
if (req.method === 'POST' && req.url === '/v2/search') {
|
||||
// The developer category is an extra arm: the API returns its hits in a
|
||||
// `data.developer` group beside the web results.
|
||||
@@ -488,27 +496,22 @@ test('search surface requires authentication for tools/list', async (t) => {
|
||||
assert.match(wwwAuthenticate, /error="invalid_token"/);
|
||||
});
|
||||
|
||||
test('search surface rejects an invalid raw API credential during tools/list', async (t) => {
|
||||
test('search surface admits a well-formed API key without resolving it first', async (t) => {
|
||||
const { searchPort } = await startHostedServer(t);
|
||||
|
||||
// Core authenticates the key on every call, so listing tools no longer waits
|
||||
// on the authorization server. The verdict arrives when a tool runs.
|
||||
const res = await jsonRpc(searchPort, SEARCH_ENDPOINT, {
|
||||
id: 8,
|
||||
method: 'tools/list',
|
||||
headers: { 'x-api-key': 'fc-invalid' },
|
||||
});
|
||||
assert.equal(res.status, 401);
|
||||
assert.equal(res.headers.has('www-authenticate'), false);
|
||||
const body = await res.json();
|
||||
assert.equal(body.error, 'invalid_api_key');
|
||||
assert.equal(body.code, 'CREDENTIAL_INVALID');
|
||||
assert.equal(
|
||||
body.error_description,
|
||||
INVALID_API_KEY_MESSAGE
|
||||
);
|
||||
assert.equal(body.next_actions, undefined);
|
||||
assert.equal(res.status, 200);
|
||||
const message = parseSseJson(await res.text());
|
||||
assert.ok((message.result?.tools?.length ?? 0) > 0);
|
||||
});
|
||||
|
||||
test('search surface rejects an invalid raw API credential before a tool call', async (t) => {
|
||||
test('search surface turns a Core credential rejection into agent-legible recovery', async (t) => {
|
||||
const backend = await startFakeBackend();
|
||||
t.after(() => backend.close());
|
||||
const { searchPort } = await startHostedServer(t, {
|
||||
@@ -524,17 +527,17 @@ test('search surface rejects an invalid raw API credential before a tool call',
|
||||
},
|
||||
headers: { 'x-api-key': 'fc-invalid' },
|
||||
});
|
||||
assert.equal(res.status, 401);
|
||||
// A 200 isError result reaches the model; a transport 401 would not.
|
||||
assert.equal(res.status, 200);
|
||||
assert.equal(res.headers.has('www-authenticate'), false);
|
||||
const body = await res.json();
|
||||
assert.equal(body.error, 'invalid_api_key');
|
||||
assert.equal(body.code, 'CREDENTIAL_INVALID');
|
||||
assert.equal(
|
||||
body.error_description,
|
||||
INVALID_API_KEY_MESSAGE
|
||||
);
|
||||
assert.equal(body.next_actions, undefined);
|
||||
assert.equal(backend.requests.some((r) => r.url === '/v2/search'), false);
|
||||
const result = parseSseJson(await res.text()).result;
|
||||
assert.equal(result.isError, true);
|
||||
assert.equal(result.content[0].text, INVALID_API_KEY_MESSAGE);
|
||||
assert.equal(result.structuredContent.code, 'CREDENTIAL_INVALID');
|
||||
assert.equal(result.structuredContent.message, INVALID_API_KEY_MESSAGE);
|
||||
assert.equal(result.structuredContent.next_actions, undefined);
|
||||
// The key reached Core, which is what produced the verdict.
|
||||
assert.equal(backend.requests.some((r) => r.url === '/v2/search'), true);
|
||||
});
|
||||
|
||||
test('search surface serves path-scoped protected-resource metadata', async (t) => {
|
||||
@@ -915,12 +918,15 @@ test('companion telemetry follows credential precedence and rejects invalid cred
|
||||
authorization: 'Bearer fco_secondary-credential',
|
||||
'x-api-key': 'fc-primary-credential',
|
||||
});
|
||||
// A well-formed key is admitted here and judged by Core, so the connect-time
|
||||
// outcome is accepted for both. Precedence and sanitization are what this
|
||||
// record exists to prove.
|
||||
const invalid = await jsonRpc(searchPort, SEARCH_ENDPOINT, {
|
||||
id: 12,
|
||||
method: 'tools/list',
|
||||
headers: { authorization: 'Bearer fc-invalid-credential' },
|
||||
});
|
||||
assert.equal(invalid.status, 401);
|
||||
assert.equal(invalid.status, 200);
|
||||
await delay(25);
|
||||
|
||||
const events = getStdout()
|
||||
@@ -931,7 +937,7 @@ test('companion telemetry follows credential precedence and rejects invalid cred
|
||||
events.map(({ auth_mode, outcome }) => ({ auth_mode, outcome })),
|
||||
[
|
||||
{ auth_mode: 'api-key', outcome: 'accepted' },
|
||||
{ auth_mode: 'api-key', outcome: 'rejected' },
|
||||
{ auth_mode: 'api-key', outcome: 'accepted' },
|
||||
]
|
||||
);
|
||||
assert.doesNotMatch(getStdout(), /fc-primary-credential|fco_secondary-credential/);
|
||||
|
||||
+42
-19
@@ -372,6 +372,14 @@ async function startFakeFirecrawlBackend(options = {}) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Core is the authority on a forwarded API key, so the suite's invalid keys
|
||||
// are rejected here rather than at introspection.
|
||||
if ((req.headers.authorization ?? '').includes('invalid')) {
|
||||
res.writeHead(401, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify({ error: 'Unauthorized: Invalid token', success: false }));
|
||||
return;
|
||||
}
|
||||
|
||||
if (req.method === 'POST' && req.url === '/v2/search') {
|
||||
if (searchResponse) {
|
||||
res.writeHead(searchResponse.status, { 'content-type': 'application/json' });
|
||||
@@ -1909,9 +1917,8 @@ test('credential validation outages do not misdirect clients into OAuth', async
|
||||
});
|
||||
await waitForHealth(port, child);
|
||||
|
||||
for (const token of ['fc-account-key', 'fco_account_token']) {
|
||||
stderr = '';
|
||||
const response = await fetch(`http://127.0.0.1:${port}/v2/mcp-oauth`, {
|
||||
const listTools = (token) =>
|
||||
fetch(`http://127.0.0.1:${port}/v2/mcp-oauth`, {
|
||||
body: JSON.stringify({ id: token, jsonrpc: '2.0', method: 'tools/list', params: {} }),
|
||||
headers: {
|
||||
accept: 'application/json, text/event-stream',
|
||||
@@ -1920,18 +1927,30 @@ test('credential validation outages do not misdirect clients into OAuth', async
|
||||
},
|
||||
method: 'POST',
|
||||
});
|
||||
await assertCredentialValidationUnavailable(response, token);
|
||||
|
||||
// An unreachable issuer is a transport fault, not the abort budget firing.
|
||||
const record = await waitForCredentialValidationLog(() => stderr);
|
||||
assert.ok(record, `${token}: missing validation log in ${stderr}`);
|
||||
assert.equal(record.reason, 'introspect_transport_error', token);
|
||||
assert.equal(record.introspect_status, null, token);
|
||||
assert.equal(record.aborted, false, token);
|
||||
assert.equal(record.resource, ACCOUNT_RESOURCE, token);
|
||||
assert.equal(typeof record.elapsed_ms, 'number', token);
|
||||
assert.doesNotMatch(stderr, new RegExp(token), token);
|
||||
}
|
||||
// An OAuth access token cannot be resolved without the issuer, so it still
|
||||
// fails closed.
|
||||
stderr = '';
|
||||
const oauth = await listTools('fco_account_token');
|
||||
await assertCredentialValidationUnavailable(oauth, 'fco_account_token');
|
||||
|
||||
// An unreachable issuer is a transport fault, not the abort budget firing.
|
||||
const record = await waitForCredentialValidationLog(() => stderr);
|
||||
assert.ok(record, `missing validation log in ${stderr}`);
|
||||
assert.equal(record.reason, 'introspect_transport_error');
|
||||
assert.equal(record.introspect_status, null);
|
||||
assert.equal(record.aborted, false);
|
||||
assert.equal(record.resource, ACCOUNT_RESOURCE);
|
||||
assert.equal(typeof record.elapsed_ms, 'number');
|
||||
assert.doesNotMatch(stderr, /fco_account_token/);
|
||||
|
||||
// An API key never needed the issuer, so the same outage does not reach it.
|
||||
stderr = '';
|
||||
const apiKey = await listTools('fc-account-key');
|
||||
assert.equal(apiKey.status, 200);
|
||||
assert.match(await apiKey.text(), /firecrawl_scrape/);
|
||||
await delay(150);
|
||||
assert.doesNotMatch(stderr, /\[MCP_CREDENTIAL_VALIDATION\]/);
|
||||
});
|
||||
|
||||
test('active introspection with an unknown credential purpose fails closed', async (t) => {
|
||||
@@ -2489,10 +2508,14 @@ test('account OAuth tokens cannot replay on keyless and invalid keys get correct
|
||||
const invalidLegacyJson = parseSseJson(await invalidLegacyPath.text());
|
||||
assert.ok((invalidLegacyJson.result?.tools?.length ?? 0) > 0);
|
||||
await delay(25);
|
||||
const rejectedTelemetry = stdout
|
||||
// A well-formed API key is admitted at connect time now that Core owns the
|
||||
// verdict, so the legacy-path record reports accepted and the correction is
|
||||
// delivered by the tool call above. The record must still name no credential.
|
||||
const legacyTelemetry = stdout
|
||||
.split(/\r?\n/)
|
||||
.find((line) => line.includes('[MCP_LEGACY_KEY_PATH]') && line.includes('\"outcome\":\"rejected\"'));
|
||||
assert.ok(rejectedTelemetry, stdout);
|
||||
assert.doesNotMatch(rejectedTelemetry, /\bfc-[^\s"]+/);
|
||||
assert.doesNotMatch(rejectedTelemetry, /(?:\d{1,3}\.){3}\d{1,3}|::1/);
|
||||
.find((line) => line.includes('[MCP_LEGACY_KEY_PATH]'));
|
||||
assert.ok(legacyTelemetry, stdout);
|
||||
assert.match(legacyTelemetry, /"outcome":"accepted"/);
|
||||
assert.doesNotMatch(legacyTelemetry, /\bfc-[^\s"]+/);
|
||||
assert.doesNotMatch(legacyTelemetry, /(?:\d{1,3}\.){3}\d{1,3}|::1/);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user