mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: address Codex round 5 (token out of DOM, raw server name, re-handshake)
- Shim removes its own <script> element before any model script runs, so the handshake token can't be read out of the DOM (document.scripts[...]) and replayed after a self-navigation. (P1 — closes the gap in the round-4 token.) - Bridge resolves the RAW configured server name from the normalized tool-key suffix (normalizeServerName) for mcpConfig/connection/tool-def lookup and rebuilds the tool key with it, so servers with special-char names work. - Re-key the live iframe by fileId+reloadNonce so switching between two live artifacts with identical HTML still remounts and re-handshakes (no stale port).
This commit is contained in:
parent
5098e9a8f6
commit
7c81966a82
4 changed files with 33 additions and 12 deletions
|
|
@ -1,6 +1,11 @@
|
|||
const { nanoid } = require('nanoid');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { checkAccess, loadWebSearchAuth, authorizeArtifactToolCall } = require('@librechat/api');
|
||||
const {
|
||||
checkAccess,
|
||||
loadWebSearchAuth,
|
||||
normalizeServerName,
|
||||
authorizeArtifactToolCall,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
Tools,
|
||||
AuthType,
|
||||
|
|
@ -317,7 +322,7 @@ const callArtifactTool = async (req, res) => {
|
|||
res.status(403).json({ message: 'Tool not permitted for this artifact' });
|
||||
return;
|
||||
}
|
||||
const { serverName } = authorization;
|
||||
const { serverName: normalizedServerName, toolName } = authorization;
|
||||
|
||||
const hasAccess = await userCanUseMCPServers(req.user, req);
|
||||
if (!hasAccess) {
|
||||
|
|
@ -330,15 +335,21 @@ const callArtifactTool = async (req, res) => {
|
|||
req.user.id,
|
||||
{ role: req.user.role, tenantId: req.user.tenantId },
|
||||
);
|
||||
const serverConfig = mcpConfig[serverName];
|
||||
/* Tool keys expose a NORMALIZED server name; `mcpConfig` is keyed by the raw
|
||||
* configured name. Resolve back to the raw name so lookup/connection/tool
|
||||
* resolution all use the same key the normal agent path uses. */
|
||||
const rawServerName = mcpConfig[normalizedServerName]
|
||||
? normalizedServerName
|
||||
: Object.keys(mcpConfig).find((name) => normalizeServerName(name) === normalizedServerName);
|
||||
const serverConfig = rawServerName ? mcpConfig[rawServerName] : undefined;
|
||||
if (!serverConfig) {
|
||||
res.status(404).json({ message: `MCP server "${serverName}" not found` });
|
||||
res.status(404).json({ message: `MCP server "${normalizedServerName}" not found` });
|
||||
return;
|
||||
}
|
||||
|
||||
const { connectionState, requiresOAuth } = await getServerConnectionStatus(
|
||||
req.user.id,
|
||||
serverName,
|
||||
rawServerName,
|
||||
serverConfig,
|
||||
appConnections,
|
||||
userConnections,
|
||||
|
|
@ -346,8 +357,8 @@ const callArtifactTool = async (req, res) => {
|
|||
);
|
||||
if (connectionState !== 'connected') {
|
||||
res.status(409).json({
|
||||
message: `MCP server "${serverName}" is not connected`,
|
||||
serverName,
|
||||
message: `MCP server "${rawServerName}" is not connected`,
|
||||
serverName: rawServerName,
|
||||
connectionState,
|
||||
requiresOAuth,
|
||||
});
|
||||
|
|
@ -357,7 +368,7 @@ const callArtifactTool = async (req, res) => {
|
|||
/* Resolve the user's per-server custom variables (API keys, etc.) the same
|
||||
* way the normal agent path does, so servers that template `{{USER_VAR}}`
|
||||
* aren't invoked without the user's configured values. */
|
||||
const pluginKey = `${Constants.mcp_prefix}${serverName}`;
|
||||
const pluginKey = `${Constants.mcp_prefix}${rawServerName}`;
|
||||
const customUserVars = {};
|
||||
if (serverConfig.customUserVars && typeof serverConfig.customUserVars === 'object') {
|
||||
for (const varName of Object.keys(serverConfig.customUserVars)) {
|
||||
|
|
@ -377,13 +388,15 @@ const callArtifactTool = async (req, res) => {
|
|||
* per user/server and would otherwise return an unavailable stub). */
|
||||
const availableTools = await getMCPManager(req.user.id).getServerToolFunctions(
|
||||
req.user.id,
|
||||
serverName,
|
||||
rawServerName,
|
||||
);
|
||||
|
||||
const toolInstance = await createMCPTool({
|
||||
res: createNoopEventSink(),
|
||||
user: req.user,
|
||||
toolKey: tool,
|
||||
// Rebuild the tool key with the RAW server name so createMCPTool resolves
|
||||
// the connection/definition the same way the agent path does.
|
||||
toolKey: `${toolName}${Constants.mcp_delimiter}${rawServerName}`,
|
||||
config: serverConfig,
|
||||
userMCPAuthMap,
|
||||
availableTools,
|
||||
|
|
|
|||
|
|
@ -193,7 +193,7 @@ export default function LiveArtifactPreview({
|
|||
</div>
|
||||
|
||||
<iframe
|
||||
key={reloadNonce}
|
||||
key={`${fileId}:${reloadNonce}`}
|
||||
ref={iframeRef}
|
||||
srcDoc={srcDocument}
|
||||
sandbox="allow-scripts"
|
||||
|
|
|
|||
|
|
@ -33,11 +33,14 @@ describe('buildLiveArtifactDocument', () => {
|
|||
expect(doc.indexOf('window.librechat')).toBeLessThan(doc.indexOf('<h1>hi</h1>'));
|
||||
});
|
||||
|
||||
it('embeds the handshake token in the shim', () => {
|
||||
it('embeds the handshake token in a self-removing shim', () => {
|
||||
const doc = buildLiveArtifactDocument('<h1>hi</h1>', 'secret-tok');
|
||||
expect(doc).toContain('secret-tok');
|
||||
expect(doc).toContain('librechat:ready');
|
||||
expect(doc).toContain('librechat:ack');
|
||||
// The shim removes its own element so the token can't be read from the DOM.
|
||||
expect(doc).toContain('document.currentScript');
|
||||
expect(doc).toMatch(/removeChild\(self\)/);
|
||||
});
|
||||
|
||||
it('puts CSP + shim before authored markup even when content has a pre-<head> prefix', () => {
|
||||
|
|
|
|||
|
|
@ -56,6 +56,11 @@ const CONTENT_SECURITY_POLICY = [
|
|||
*/
|
||||
const buildBridgeShim = (token: string): string => `
|
||||
(function () {
|
||||
// Remove this script element before any model-authored script can run, so the
|
||||
// token literal in its source can't be read out of the DOM (e.g. via
|
||||
// document.scripts[...].textContent) and replayed after a self-navigation.
|
||||
var self = document.currentScript;
|
||||
if (self && self.parentNode) self.parentNode.removeChild(self);
|
||||
var TOKEN = ${JSON.stringify(token)};
|
||||
var pending = {};
|
||||
var seq = 0;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue