mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-10 16:23:44 +00:00
🤐 fix: Exclude Provider Secrets from HITL Pending Actions (#14136)
* 🤐 fix: Exclude Provider Secrets from HITL Pending Actions Sanitize resolved model parameters before persisting them in the pending action's resumeContext, and strip resumeContext/requestFingerprint from every client-facing copy (SSE emit, reconnect gap-fill, resume state, status route). The full record stays server-side for resume replay. * 🤐 fix: Treat Header-Carrier Keys as Sensitive Wholesale Google llmConfig places the Authorization header in customHeaders, and header names like Ocp-Apim-Subscription-Key defeat name heuristics. Match 'header' as a key fragment so every header-carrier object is dropped rather than relying on exact carrier names. * 🤐 fix: Strip Preconfigured Client Instances from Resume Params Bedrock stores a BedrockRuntimeClient on llmConfig.client when PROXY or a bearer token is configured; replaying it would also fold a mangled client object into additionalModelRequestFields on resume.
This commit is contained in:
parent
47a8749428
commit
dcdaaeac67
6 changed files with 275 additions and 9 deletions
|
|
@ -34,8 +34,10 @@ const {
|
|||
getTransactionsConfig,
|
||||
resolveRecursionLimit,
|
||||
buildPendingAction,
|
||||
toClientPendingAction,
|
||||
computeAgentRequestFingerprint,
|
||||
extractDiscoveredToolsFromHistory,
|
||||
sanitizeResumeModelParameters,
|
||||
pickResumeContext,
|
||||
getApprovalTtlMs,
|
||||
isHITLEnabled,
|
||||
|
|
@ -1222,9 +1224,13 @@ class AgentClient extends BaseClient {
|
|||
// paused on. The resume payload omits them and they aren't part of the fingerprint, so
|
||||
// without this the rebuilt ephemeral run falls back to defaults. (Saved agents source
|
||||
// these from the DB record server-side, so this is belt-and-suspenders for them.)
|
||||
// Sanitized: the resolved params are the llmConfig, which carries provider secrets
|
||||
// (apiKey, credentials) and gateway config — resume re-resolves those server-side.
|
||||
const resumeContext = pickResumeContext(this.options.req?.body);
|
||||
const resolvedModelParameters = this.options.agent?.model_parameters;
|
||||
if (resolvedModelParameters && typeof resolvedModelParameters === 'object') {
|
||||
const resolvedModelParameters = sanitizeResumeModelParameters(
|
||||
this.options.agent?.model_parameters,
|
||||
);
|
||||
if (resolvedModelParameters) {
|
||||
resumeContext.model_parameters = resolvedModelParameters;
|
||||
}
|
||||
const pendingAction = buildPendingAction(interrupt.payload, {
|
||||
|
|
@ -1311,7 +1317,7 @@ class AgentClient extends BaseClient {
|
|||
}
|
||||
await GenerationJobManager.emitChunk(streamId, {
|
||||
event: ApprovalEvents.ON_PENDING_ACTION,
|
||||
data: pendingAction,
|
||||
data: toClientPendingAction(pendingAction),
|
||||
});
|
||||
logger.debug(
|
||||
`[AgentClient] Paused ${streamId} for ${interrupt.payload.type} (action ${pendingAction.actionId})`,
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ const {
|
|||
hasPersistableAbortContent,
|
||||
buildAbortedResponseMetadata,
|
||||
isPendingActionStale,
|
||||
toClientPendingAction,
|
||||
isHITLEnabled,
|
||||
deleteAgentCheckpoint,
|
||||
} = require('@librechat/api');
|
||||
|
|
@ -222,8 +223,12 @@ router.get('/chat/status/:conversationId', async (req, res) => {
|
|||
resumeState,
|
||||
// Surface the live pending approval so a client rebuilding from /chat/status
|
||||
// (reload / cross-replica) has the action id + payload to render and submit
|
||||
// the prompt, not just the knowledge that the stream is paused.
|
||||
pendingAction: job.status === 'requires_action' && pendingLive ? pendingAction : undefined,
|
||||
// the prompt, not just the knowledge that the stream is paused. Client-safe
|
||||
// projection only — resumeContext/requestFingerprint stay server-side.
|
||||
pendingAction:
|
||||
job.status === 'requires_action' && pendingLive
|
||||
? toClientPendingAction(pendingAction)
|
||||
: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -6,7 +6,9 @@ import {
|
|||
buildToolApprovalPayload,
|
||||
buildAskUserQuestionPayload,
|
||||
buildPendingAction,
|
||||
toClientPendingAction,
|
||||
computeAgentRequestFingerprint,
|
||||
sanitizeResumeModelParameters,
|
||||
pickResumeContext,
|
||||
applyResumeContext,
|
||||
} from './policy';
|
||||
|
|
@ -257,6 +259,104 @@ describe('buildPendingAction', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('toClientPendingAction', () => {
|
||||
const payload: Agents.ToolApprovalInterruptPayload = {
|
||||
type: 'tool_approval',
|
||||
action_requests: [{ name: 'shell', arguments: { command: 'ls' }, tool_call_id: 'call_abc' }],
|
||||
review_configs: [
|
||||
{ action_name: 'shell', tool_call_id: 'call_abc', allowed_decisions: ['approve', 'reject'] },
|
||||
],
|
||||
};
|
||||
|
||||
test('omits server-only replay state, keeping the fields the client renders from', () => {
|
||||
const full = buildPendingAction(payload, {
|
||||
streamId: 'stream-1',
|
||||
conversationId: 'conv-1',
|
||||
requestFingerprint: 'fp-hash',
|
||||
resumeContext: {
|
||||
endpoint: 'agents',
|
||||
model_parameters: { temperature: 0.5 },
|
||||
},
|
||||
});
|
||||
|
||||
const clientSafe = toClientPendingAction(full);
|
||||
expect(clientSafe).toBeDefined();
|
||||
expect(clientSafe?.resumeContext).toBeUndefined();
|
||||
expect(clientSafe?.requestFingerprint).toBeUndefined();
|
||||
expect(clientSafe?.actionId).toBe(full.actionId);
|
||||
expect(clientSafe?.streamId).toBe('stream-1');
|
||||
expect(clientSafe?.payload).toBe(full.payload);
|
||||
// Non-mutating: the stored record keeps its replay state for the resume route.
|
||||
expect(full.resumeContext).toBeDefined();
|
||||
expect(full.requestFingerprint).toBe('fp-hash');
|
||||
});
|
||||
|
||||
test('passes through nullish input', () => {
|
||||
expect(toClientPendingAction(undefined)).toBeUndefined();
|
||||
expect(toClientPendingAction(null)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('sanitizeResumeModelParameters', () => {
|
||||
test('strips provider credentials and transport config across provider shapes', () => {
|
||||
const sanitized = sanitizeResumeModelParameters({
|
||||
model: 'gpt-5',
|
||||
temperature: 0.2,
|
||||
maxTokens: 1024,
|
||||
max_tokens: 512,
|
||||
apiKey: 'sk-server-secret',
|
||||
azureOpenAIApiKey: 'azure-secret',
|
||||
azureOpenAIApiInstanceName: 'internal-resource',
|
||||
anthropicApiUrl: 'https://internal-gateway.example',
|
||||
configuration: {
|
||||
baseURL: 'https://internal-gateway.example',
|
||||
defaultHeaders: { Authorization: 'Bearer server-secret' },
|
||||
},
|
||||
clientOptions: { defaultHeaders: { 'x-api-key': 'anthropic-secret' } },
|
||||
customHeaders: { 'Ocp-Apim-Subscription-Key': 'gateway-secret' },
|
||||
authOptions: { credentials: { private_key: 'google-secret' } },
|
||||
credentials: { accessKeyId: 'aws-id', secretAccessKey: 'aws-secret' },
|
||||
client: { config: { token: { token: 'bedrock-bearer' } } },
|
||||
endpointHost: 'vpce.internal.example',
|
||||
baseURL: 'https://internal-gateway.example',
|
||||
});
|
||||
|
||||
expect(sanitized).toEqual({
|
||||
model: 'gpt-5',
|
||||
temperature: 0.2,
|
||||
maxTokens: 1024,
|
||||
max_tokens: 512,
|
||||
});
|
||||
});
|
||||
|
||||
test('keeps user-level params while stripping nested secret keys from custom params', () => {
|
||||
const sanitized = sanitizeResumeModelParameters({
|
||||
maxTokens: 2048,
|
||||
stop: ['a', 'b'],
|
||||
custom: { safe: true, api_key: 'x', token: 'y' },
|
||||
});
|
||||
|
||||
expect(sanitized).toEqual({
|
||||
maxTokens: 2048,
|
||||
stop: ['a', 'b'],
|
||||
custom: { safe: true },
|
||||
});
|
||||
});
|
||||
|
||||
test('drops function values and returns undefined for non-object input', () => {
|
||||
const sanitized = sanitizeResumeModelParameters({
|
||||
temperature: 1,
|
||||
fetch: () => undefined,
|
||||
});
|
||||
expect(sanitized).toEqual({ temperature: 1 });
|
||||
|
||||
expect(sanitizeResumeModelParameters(undefined)).toBeUndefined();
|
||||
expect(sanitizeResumeModelParameters(null)).toBeUndefined();
|
||||
expect(sanitizeResumeModelParameters('sk-secret')).toBeUndefined();
|
||||
expect(sanitizeResumeModelParameters(['sk-secret'])).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('computeAgentRequestFingerprint', () => {
|
||||
it('is stable for the same graph-determining fields (ignoring other body keys)', () => {
|
||||
const a = computeAgentRequestFingerprint({
|
||||
|
|
|
|||
|
|
@ -238,7 +238,113 @@ export const RESUME_CONTEXT_KEYS = [
|
|||
'manualSkills',
|
||||
] as const;
|
||||
|
||||
export type ResumeContext = Partial<Record<(typeof RESUME_CONTEXT_KEYS)[number], unknown>>;
|
||||
export type ResumeContext = Partial<Record<(typeof RESUME_CONTEXT_KEYS)[number], unknown>> & {
|
||||
/** Resolved model params captured at pause (sanitized); replayed by the resume route. */
|
||||
model_parameters?: Record<string, unknown>;
|
||||
};
|
||||
|
||||
/** Exact (lowercased) parameter keys that carry credentials or server transport config. */
|
||||
const SENSITIVE_PARAM_KEYS = new Set([
|
||||
'auth',
|
||||
'authoptions',
|
||||
'auth_options',
|
||||
'token',
|
||||
'accesstoken',
|
||||
'access_token',
|
||||
'refreshtoken',
|
||||
'refresh_token',
|
||||
'idtoken',
|
||||
'id_token',
|
||||
'sessiontoken',
|
||||
'session_token',
|
||||
'configuration',
|
||||
'client',
|
||||
'clientoptions',
|
||||
'client_options',
|
||||
'fetchoptions',
|
||||
'fetch_options',
|
||||
'fetch',
|
||||
'httpagent',
|
||||
'httpsagent',
|
||||
'callbacks',
|
||||
'endpointhost',
|
||||
'endpoint_host',
|
||||
]);
|
||||
|
||||
/** Key fragments matched anywhere in a (lowercased) key, e.g. `azureOpenAIApiKey`. */
|
||||
const SENSITIVE_PARAM_KEY_FRAGMENTS = [
|
||||
'apikey',
|
||||
'api_key',
|
||||
'api-key',
|
||||
'apiurl',
|
||||
'api_url',
|
||||
'api-url',
|
||||
'secret',
|
||||
'password',
|
||||
'credential',
|
||||
'authorization',
|
||||
'azureopenai',
|
||||
'header',
|
||||
'proxy',
|
||||
'baseurl',
|
||||
'base_url',
|
||||
'basepath',
|
||||
'base_path',
|
||||
];
|
||||
|
||||
function isSensitiveParamKey(key: string): boolean {
|
||||
const normalized = key.toLowerCase();
|
||||
if (SENSITIVE_PARAM_KEYS.has(normalized)) {
|
||||
return true;
|
||||
}
|
||||
return SENSITIVE_PARAM_KEY_FRAGMENTS.some((fragment) => normalized.includes(fragment));
|
||||
}
|
||||
|
||||
/** Bounded recursion guard for pathological / cyclic parameter graphs. */
|
||||
const MAX_SANITIZE_DEPTH = 8;
|
||||
|
||||
function sanitizeParamValue(value: unknown, depth: number): unknown {
|
||||
if (typeof value === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return depth >= MAX_SANITIZE_DEPTH
|
||||
? undefined
|
||||
: value.map((item) => sanitizeParamValue(item, depth + 1));
|
||||
}
|
||||
if (value != null && typeof value === 'object') {
|
||||
if (depth >= MAX_SANITIZE_DEPTH) {
|
||||
return undefined;
|
||||
}
|
||||
const sanitized: Record<string, unknown> = {};
|
||||
for (const [key, child] of Object.entries(value as Record<string, unknown>)) {
|
||||
if (isSensitiveParamKey(key) || typeof child === 'function') {
|
||||
continue;
|
||||
}
|
||||
sanitized[key] = sanitizeParamValue(child, depth + 1);
|
||||
}
|
||||
return sanitized;
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip credentials and server transport config from resolved model parameters before
|
||||
* they are persisted for resume replay. The initialized agent's `model_parameters` are
|
||||
* the resolved `llmConfig` — they carry provider secrets (`apiKey`, Azure key names,
|
||||
* Google `authOptions`, Bedrock `credentials`) and gateway config (`configuration`,
|
||||
* headers, base URLs). Resume re-resolves all of those server-side from env/config, so
|
||||
* only the user-level generation params (temperature, max tokens, custom endpoint
|
||||
* params, …) need to survive the round trip.
|
||||
*/
|
||||
export function sanitizeResumeModelParameters(
|
||||
params: unknown,
|
||||
): Record<string, unknown> | undefined {
|
||||
if (params == null || typeof params !== 'object' || Array.isArray(params)) {
|
||||
return undefined;
|
||||
}
|
||||
return sanitizeParamValue(params, 0) as Record<string, unknown>;
|
||||
}
|
||||
|
||||
/** Extract the graph-determining fields from a request body for durable replay. */
|
||||
export function pickResumeContext(body: Record<string, unknown> | undefined | null): ResumeContext {
|
||||
|
|
@ -319,3 +425,23 @@ export function buildPendingAction(
|
|||
resumeContext: ctx.resumeContext,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Client-facing projection of a pending action. `requestFingerprint` and `resumeContext`
|
||||
* are server-only replay state — `resumeContext` in particular carries the resolved
|
||||
* model parameters — so every copy that leaves the server (SSE, status, resume state)
|
||||
* must go through this. The full record stays in the job store for the resume route.
|
||||
*/
|
||||
export function toClientPendingAction(
|
||||
pendingAction: Agents.PendingAction | undefined | null,
|
||||
): Agents.PendingAction | undefined {
|
||||
if (pendingAction == null) {
|
||||
return undefined;
|
||||
}
|
||||
const {
|
||||
requestFingerprint: _requestFingerprint,
|
||||
resumeContext: _resumeContext,
|
||||
...clientSafe
|
||||
} = pendingAction;
|
||||
return clientSafe;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import { isPendingActionStale, isPendingActionExpired } from './interfaces/IJobS
|
|||
import { InMemoryEventTransport } from './implementations/InMemoryEventTransport';
|
||||
import { InMemoryJobStore } from './implementations/InMemoryJobStore';
|
||||
import { filterPersistableAbortContent } from './abortContent';
|
||||
import { toClientPendingAction } from '~/agents/hitl/policy';
|
||||
import { ApprovalLifecycle } from './ApprovalLifecycle';
|
||||
|
||||
/** Terminal error surfaced to a client still attached when its approval window lapses. */
|
||||
|
|
@ -1107,7 +1108,10 @@ class GenerationJobManagerClass {
|
|||
...pendingEvents,
|
||||
{
|
||||
event: ApprovalEvents.ON_PENDING_ACTION,
|
||||
data: liveJob.pendingAction as unknown as Record<string, unknown>,
|
||||
data: toClientPendingAction(liveJob.pendingAction) as unknown as Record<
|
||||
string,
|
||||
unknown
|
||||
>,
|
||||
},
|
||||
];
|
||||
}
|
||||
|
|
@ -1612,10 +1616,11 @@ class GenerationJobManagerClass {
|
|||
collectedUsage,
|
||||
contextUsage,
|
||||
// Carry the live pending approval in the resume contract so a reloading /
|
||||
// cross-replica client can rebuild the prompt from resumeState.
|
||||
// cross-replica client can rebuild the prompt from resumeState. Client-safe
|
||||
// projection: the stored record's resumeContext/requestFingerprint stay server-only.
|
||||
pendingAction:
|
||||
jobData.status === 'requires_action' && !isPendingActionStale(jobData)
|
||||
? jobData.pendingAction
|
||||
? toClientPendingAction(jobData.pendingAction)
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -67,6 +67,30 @@ describe('ApprovalLifecycle via GenerationJobManager.approvals (in-memory)', ()
|
|||
test('returns false when the job does not exist', async () => {
|
||||
expect(await manager.approvals.pause('nonexistent', buildAction('nonexistent'))).toBe(false);
|
||||
});
|
||||
|
||||
test('client-facing copies omit resumeContext/requestFingerprint; the stored record keeps them', async () => {
|
||||
const streamId = 'stream-redact';
|
||||
await manager.createJob(streamId, 'user-1');
|
||||
|
||||
const action = buildAction(streamId, {
|
||||
requestFingerprint: 'fp-hash',
|
||||
resumeContext: {
|
||||
endpoint: 'agents',
|
||||
model_parameters: { temperature: 0.7 },
|
||||
},
|
||||
});
|
||||
expect(await manager.approvals.pause(streamId, action)).toBe(true);
|
||||
|
||||
const resumeState = await manager.getResumeState(streamId);
|
||||
expect(resumeState?.pendingAction?.actionId).toBe(action.actionId);
|
||||
expect(resumeState?.pendingAction?.resumeContext).toBeUndefined();
|
||||
expect(resumeState?.pendingAction?.requestFingerprint).toBeUndefined();
|
||||
|
||||
// The resume route replays from the job store, which must retain the full record.
|
||||
const job = await manager.getJob(streamId);
|
||||
expect(job?.metadata.pendingAction?.resumeContext).toEqual(action.resumeContext);
|
||||
expect(job?.metadata.pendingAction?.requestFingerprint).toBe('fp-hash');
|
||||
});
|
||||
});
|
||||
|
||||
describe('peek', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue