mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-24 03:09:26 +00:00
🛡️ fix: Close Name Disclosure and Follow the PTC Trace Tail
Two findings from the third Codex pass on `17a9ec9`.
Redact filtered inner-tool names (P1). The previous gate suppressed argument
and failure previews but the event still carried `name` verbatim, so a
deployment whose `filters.toolArguments.pii.fields` includes `name` could see
a blocked identifier disclosed through the trace — the one path inner calls
take, since they never reach `filteredToolArgumentsResult`. Inner tool names
are now inspected once per PTC call with the same `extractToolArgumentContent`
+ `inspectContent` pair the executor uses; any that trip the policy are left
unwrapped, so they still execute and emit nothing. An un-inspectable name
fails closed.
Follow the trace tail (P2). The row list is a 200px scroller that never moved,
so once a program exceeded the viewport the card sat on the oldest calls while
live activity accumulated below the fold. Reuse `useFollowScroll` — the hook
the code and command panes already use — which pins to the tail while calls
are running and yields the moment the reader scrolls up. The host card threads
its disclosure state so a collapsed pane is never scrolled invisibly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP
This commit is contained in:
parent
bf29a731d0
commit
9e0be3ca30
6 changed files with 93 additions and 4 deletions
|
|
@ -179,6 +179,7 @@ export default function BashCall({
|
|||
)}
|
||||
<PtcToolTrace
|
||||
toolCallId={toolCallId}
|
||||
expanded={showCode}
|
||||
className={cn(command && 'border-t border-border-light')}
|
||||
/>
|
||||
{hasOutput && backgroundHandle == null && (
|
||||
|
|
|
|||
|
|
@ -180,6 +180,7 @@ export default function ExecuteCode({
|
|||
)}
|
||||
<PtcToolTrace
|
||||
toolCallId={toolCallId}
|
||||
expanded={showCode}
|
||||
className={cn(code && 'border-t border-border-light')}
|
||||
/>
|
||||
{hasOutput && backgroundHandle == null && (
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { cn, parseToolName, getRunStepDurationLabels } from '~/utils';
|
|||
import { useMessageContext } from '~/Providers/MessageContext';
|
||||
import { ptcTraceByToolCallId, ptcTraceKey } from '~/store';
|
||||
import { useMCPServerNames } from '~/hooks/MCP';
|
||||
import useFollowScroll from './useFollowScroll';
|
||||
import { useLocalize } from '~/hooks';
|
||||
|
||||
/** Terminal-style status column: one glyph wide for every state, so the
|
||||
|
|
@ -105,9 +106,13 @@ function PtcTraceLine({ entry }: { entry: PtcTraceEntry }) {
|
|||
*/
|
||||
export default function PtcToolTrace({
|
||||
toolCallId,
|
||||
expanded = false,
|
||||
className,
|
||||
}: {
|
||||
toolCallId?: string;
|
||||
/** The host card's disclosure state — a collapsed pane must not be scrolled
|
||||
* invisibly, or it opens at the tail instead of the first call. */
|
||||
expanded?: boolean;
|
||||
className?: string;
|
||||
}) {
|
||||
const localize = useLocalize();
|
||||
|
|
@ -118,6 +123,16 @@ export default function PtcToolTrace({
|
|||
ptcTraceByToolCallId(toolCallId && messageId ? ptcTraceKey(messageId, toolCallId) : ''),
|
||||
);
|
||||
|
||||
/** One character per row, so the pin re-fires both when a call is appended
|
||||
* and when one settles — a settle can add an error line and change height. */
|
||||
const followKey = `${trace.dropped}:${trace.entries.map((entry) => entry.status[0]).join('')}`;
|
||||
const running = trace.entries.some((entry) => entry.status === 'running');
|
||||
const { ref: listRef, onScroll } = useFollowScroll<HTMLOListElement>(
|
||||
followKey,
|
||||
running,
|
||||
expanded,
|
||||
);
|
||||
|
||||
if (trace.entries.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
|
@ -131,7 +146,12 @@ export default function PtcToolTrace({
|
|||
<div className="mb-1.5 text-[10px] font-medium uppercase tracking-wide text-text-secondary">
|
||||
{localize('com_ui_ptc_trace_title')}
|
||||
</div>
|
||||
<ol className="max-h-[200px] overflow-auto font-mono" aria-live="polite">
|
||||
<ol
|
||||
ref={listRef}
|
||||
onScroll={onScroll}
|
||||
className="max-h-[200px] overflow-auto font-mono"
|
||||
aria-live="polite"
|
||||
>
|
||||
{/* The retained tail is a window, not the whole program — say so
|
||||
rather than let the reader assume these are all the calls. */}
|
||||
{trace.dropped > 0 && (
|
||||
|
|
|
|||
|
|
@ -750,6 +750,35 @@ function filteredToolArgumentsResult(
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Inner tool names the `name` PII policy would block. `filteredToolArgumentsResult`
|
||||
* inspects `tc.name` for direct calls, but inner calls bypass it entirely — and
|
||||
* the trace event carries the name unconditionally, so without this the trace
|
||||
* becomes the disclosure path the policy exists to close. The eligible map holds
|
||||
* a handful of names, each inspected once per PTC call.
|
||||
*/
|
||||
function collectFilteredPtcToolNames(
|
||||
names: Iterable<string>,
|
||||
req: ServerRequest | undefined,
|
||||
): ReadonlySet<string> | undefined {
|
||||
const filters = req?.config?.filters;
|
||||
if (filters == null || !hasActivePiiFields(filters.toolArguments?.pii, ['name'])) {
|
||||
return undefined;
|
||||
}
|
||||
const blocked = new Set<string>();
|
||||
for (const name of names) {
|
||||
try {
|
||||
if (inspectContent(extractToolArgumentContent({ name }), { filters }) != null) {
|
||||
blocked.add(name);
|
||||
}
|
||||
} catch {
|
||||
/* An un-inspectable name is treated as blocked: fail closed. */
|
||||
blocked.add(name);
|
||||
}
|
||||
}
|
||||
return blocked.size > 0 ? blocked : undefined;
|
||||
}
|
||||
|
||||
function filteredToolOutputResult(
|
||||
tc: ToolCallRequest,
|
||||
req: ServerRequest | undefined,
|
||||
|
|
@ -5036,9 +5065,8 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand
|
|||
* the sandbox bridge invokes them directly — so when the
|
||||
* deployment filters tool arguments for PII, the trace
|
||||
* must not put their values on the wire. */
|
||||
const ptcArgumentPii = (
|
||||
mergedConfigurable?.req as ServerRequest | undefined
|
||||
)?.config?.filters?.toolArguments?.pii;
|
||||
const ptcReq = mergedConfigurable?.req as ServerRequest | undefined;
|
||||
const ptcArgumentPii = ptcReq?.config?.filters?.toolArguments?.pii;
|
||||
toolCallConfig.toolMap = emitPtcProgress
|
||||
? instrumentPtcToolMap({
|
||||
toolMap: eligiblePtcToolMap,
|
||||
|
|
@ -5051,6 +5079,10 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand
|
|||
'arguments',
|
||||
'output',
|
||||
]),
|
||||
traceExclusions: collectFilteredPtcToolNames(
|
||||
eligiblePtcToolMap.keys(),
|
||||
ptcReq,
|
||||
),
|
||||
emit: emitPtcProgress,
|
||||
})
|
||||
: eligiblePtcToolMap;
|
||||
|
|
|
|||
|
|
@ -210,6 +210,28 @@ describe('instrumentPtcToolMap', () => {
|
|||
expect(JSON.stringify(events)).not.toContain('555-01-0000');
|
||||
});
|
||||
|
||||
it('emits nothing at all for a tool whose name the policy filters', async () => {
|
||||
const { events, emit } = collect();
|
||||
const toolMap = new Map([
|
||||
['ok_tool', createTool('ok_tool', async () => 'ok')],
|
||||
['blocked_name_tool', createTool('blocked_name_tool', async () => 'ok')],
|
||||
]);
|
||||
|
||||
const instrumented = instrumentPtcToolMap({
|
||||
toolMap,
|
||||
toolCallId: 'call_1',
|
||||
traceExclusions: new Set(['blocked_name_tool']),
|
||||
emit,
|
||||
});
|
||||
|
||||
/** Excluded tools still execute — only their telemetry is suppressed. */
|
||||
await expect(instrumented.get('blocked_name_tool')?.invoke({ a: 1 })).resolves.toBe('ok');
|
||||
await instrumented.get('ok_tool')?.invoke({ a: 1 });
|
||||
|
||||
expect(events.map((e) => e.name)).toEqual(['ok_tool', 'ok_tool']);
|
||||
expect(JSON.stringify(events)).not.toContain('blocked_name_tool');
|
||||
});
|
||||
|
||||
it('runs the inner call even when the emitter throws', async () => {
|
||||
const toolMap = new Map([['read_file', createTool('read_file', async () => 'ok')]]);
|
||||
const instrumented = instrumentPtcToolMap({
|
||||
|
|
|
|||
|
|
@ -95,6 +95,14 @@ export interface InstrumentPtcToolMapParams {
|
|||
* argument that caused it. The trace still reports name, status and duration.
|
||||
*/
|
||||
includePreviews?: boolean;
|
||||
/**
|
||||
* Inner tools whose *name* trips the deployment's PII policy. The event
|
||||
* carries the tool name unconditionally, so a name the `name` filter would
|
||||
* have blocked on a direct call cannot be allowed to ride the trace instead.
|
||||
* These tools still execute — they are simply left unwrapped, so no event
|
||||
* about them is ever emitted.
|
||||
*/
|
||||
traceExclusions?: ReadonlySet<string>;
|
||||
emit: (event: PtcToolCallEvent) => void;
|
||||
}
|
||||
|
||||
|
|
@ -111,6 +119,7 @@ export function instrumentPtcToolMap({
|
|||
toolCallId,
|
||||
runId,
|
||||
includePreviews = true,
|
||||
traceExclusions,
|
||||
emit,
|
||||
}: InstrumentPtcToolMapParams): Map<string, StructuredToolInterface> {
|
||||
let sequence = 0;
|
||||
|
|
@ -125,6 +134,10 @@ export function instrumentPtcToolMap({
|
|||
|
||||
const instrumented = new Map<string, StructuredToolInterface>();
|
||||
for (const [name, tool] of toolMap) {
|
||||
if (traceExclusions?.has(name)) {
|
||||
instrumented.set(name, tool);
|
||||
continue;
|
||||
}
|
||||
instrumented.set(
|
||||
name,
|
||||
new Proxy(tool, {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue