🗝️ fix: Resolve User-Provided API Key in Agents API Flow

[ESJavadex]

Summary

• Fixes the no_user_key error when the Agents API (/api/agents/v1/chat/completions) is used with a custom endpoint configured with apiKey: "user_provided"
• The root cause was that getUserKeyValues was gated behind req.body.key (expiry timestamp), which is only sent by the UI chat flow — the Agents API sends an OpenAI-compatible body without this field
• Decouples the key fetch from the expiry check so the key is always retrieved when needed

Changes

packages/api/src/endpoints/custom/initialize.ts

  let userValues = null;
- if (expiresAt && (userProvidesKey || userProvidesURL)) {
-   checkUserKeyExpiry(expiresAt, endpoint);
+ if (userProvidesKey || userProvidesURL) {
+   if (expiresAt) {
+     checkUserKeyExpiry(expiresAt, endpoint);
+   }
    userValues = await db.getUserKeyValues({ userId: req.user?.id ?? '', name: endpoint });
  }

packages/api/src/endpoints/custom/initialize.spec.ts

• Added test: key is fetched when expiresAt is absent (Agents API flow)
• Added test: expiry is still checked when expiresAt is present (UI flow)

Test plan

• Added unit tests covering both Agents API (no expiresAt) and UI (with expiresAt) flows
• Manual test: call /api/agents/v1/chat/completions with an agent backed by a user_provided custom endpoint

Fixes #12389

[danny-avila]

Hey, thanks for the PR and for filing the issue with such a thorough root cause analysis! The fix itself is correct and well-targeted. A few things to address before it can be merged:

---

Must fix:

1. The test for the Agents API flow is missing a negative assertion. It confirms getUserKeyValues was called, but never asserts that checkUserKeyExpiry was not called. If the old guard were accidentally restored, the test would still pass. Please add:

   const { checkUserKeyExpiry } = jest.requireMock('~/utils');
   expect(checkUserKeyExpiry).not.toHaveBeenCalled();

2. No regression test for the expiry path. There should be a test that passes an expired expiresAt and confirms the function still throws EXPIRED_USER_KEY. This is the most important thing to lock down so the expiry enforcement isn't accidentally broken in a future refactor.

3. Add a comment in the production code on the if (expiresAt) block explaining that the Agents API doesn't send key in the request body, so expiry is only checked when present. Without it, a future security reviewer will flag this as a gap.

---

Nice to have:

4. The undefined as unknown as string cast in the test (expiresAt: undefined as unknown as string) is a no-op since createParams defaults it to '2099-01-01' internally anyway, and the body gets overwritten on the next line. You can just omit the expiresAt argument entirely.

5. Consider adding a test for the userProvidesURL = true, userProvidesKey = false variant in the Agents API flow, since the fix covers that case too.

---

Also worth fixing:

The nested if (expiresAt) inside if (userProvidesKey || userProvidesURL) goes against the project's never-nesting style guide. Flatten it into two sibling statements:

if (expiresAt && (userProvidesKey || userProvidesURL)) {
  checkUserKeyExpiry(expiresAt, endpoint);
}

if (userProvidesKey || userProvidesURL) {
  userValues = await db.getUserKeyValues({ userId: req.user?.id ?? '', name: endpoint });
}

Same behavior, no nesting.

[ESJavadex]

Thanks for the detailed review! All points addressed in 894073b:

Must fix:

1. ✅ Added expect(checkUserKeyExpiry).not.toHaveBeenCalled() in the Agents API flow test
2. ✅ Added regression test that passes an expired expiresAt and confirms EXPIRED_USER_KEY is thrown (also asserts getUserKeyValues is NOT called after the throw)
3. ✅ Added inline comment on the if (expiresAt) block explaining the Agents API context

Nice to have:
4. ✅ Removed the undefined as unknown as string cast — body is overwritten on the next line anyway
5. ✅ Added test for userProvidesURL = true, userProvidesKey = false variant in the Agents API flow

Style:
6. ✅ Flattened into two sibling statements — no nesting

[danny-avila]

One test gap remains: there's no assertion that getUserKeyValues is not called when the endpoint uses a fully system-configured key (userProvidesKey = false, userProvidesURL = false). The SSRF test suite has a 'should NOT call validateEndpointURL when baseURL is system-defined' test that could easily double as this, but getUserKeyValues is never asserted there either.

One minor nit: the expired key test uses the raw string 'expired_user_key' instead of ErrorTypes.EXPIRED_USER_KEY. Small inconsistency with how the rest of the codebase references error types.

Bottom line: All the MAJOR findings are resolved. The remaining items are small. This is close to merge-ready pending those two tweaks.

[ESJavadex]

Pushed 01fe3d8 addressing the remaining items + fixing the CI failure:

1. ✅ ErrorTypes.EXPIRED_USER_KEY instead of raw string
2. ✅ System key test: asserts getUserKeyValues is NOT called when both key and URL are system-defined
3. 🐛 CI fix: the expired key test used mockImplementation which leaked into the SSRF describe block — clearAllMocks only clears calls/results, not implementations. Switched to mockImplementationOnce.

[danny-avila]

thanks!