Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions packages/api/src/middleware/modelBoundContent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,74 @@ describe('assertModelBoundContent', () => {
).toThrow('Submitted content contains a private context');
});

it('does not traverse unrelated initialized tool schemas while checking agent files', () => {
const hydratedFile = {
file_id: 'file-agent-context',
filename: 'context.txt',
filepath: '/uploads/context.txt',
text: 'safe canonical context',
};

expect(() =>
assertModelBoundContent({
filters,
agents: [
{
instructions: 'Safe agent instructions',
toolDefinitions: [
{
name: 'deep_tool',
description: 'Safe tool description',
parameters: makeDeepModelParameter(),
},
],
requestAttachments: [hydratedFile],
} as never,
],
}),
).not.toThrow();
});

it('still checks provider-bound agent fields for opaque file content', () => {
const failClosedFilters: FiltersConfig = {
files: {
pii: {
fields: ['content'],
starterPatterns: [],
uninspectable: 'block',
},
},
};
const hydratedFile = {
file_id: 'file-agent-context',
filename: 'context.txt',
filepath: '/uploads/context.txt',
content: 'safe canonical context',
};

for (const agent of [
{
instructions: 'data:application/pdf;base64,opaque',
requestAttachments: [hydratedFile],
},
{
model_parameters: {
additionalModelRequestFields: {
input_file: { file_data: 'data:application/pdf;base64,opaque' },
},
},
requestAttachments: [hydratedFile],
},
]) {
expect(() =>
assertModelBoundContent({
filters: failClosedFilters,
agents: [agent as never],
}),
).toThrow('Submitted file content could not be inspected before processing.');
}
});

it('accepts only agent resource IDs backed by inspectable hydrated resource files', () => {
const filters: FiltersConfig = {
files: {
Expand Down
2 changes: 2 additions & 0 deletions packages/api/src/middleware/modelBoundContent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ const MAX_PROVIDER_PROVENANCE_PARSE_WORK =
MAX_PROVIDER_PROVENANCE_INDEX_REFS + MAX_PROVIDER_PROVENANCE_PARTS;
const MAX_PROVIDER_STORED_STATE_WORK =
MAX_PROVIDER_PROJECTION_WORK + MAX_PROVIDER_PROVENANCE_PARTS * 2;
const AGENT_FILE_INSPECTION_OMITTED_ROOT_KEYS = new Set(['toolDefinitions']);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Inspect caller-supplied tool definitions before omitting schemas

With files.pii.uninspectable: block, a Responses API caller can place an opaque file payload such as data:application/pdf;base64,... in a function tool's description or parameters and have it sent to the provider whenever the agent also has a hydrated attachment. Fresh evidence beyond the earlier thread is that api/server/controllers/agents/responses.js:1136-1145 merges request.tools into primaryConfig.toolDefinitions after the initial check at line 1078, while the post-merge check at line 1196 reaches this new omission; extractAgentContent then subjects the schema only to tool-argument/agent-instruction policies, not the file policy. Preserve opaque-file validation for caller-supplied definitions before omitting trusted initialized schemas.

Useful? React with 馃憤聽/ 馃憥.

/** One root plus bounded structural bookkeeping for every valid provider part. */
const MAX_MODEL_BOUND_NESTED_TRAVERSAL_WORK = CONTENT_TRAVERSAL_MAX_NODES * 2;

Expand Down Expand Up @@ -3880,6 +3881,7 @@ function inspectModelBoundContent(
assertInspectableFileInput(
input.filters,
omitResolvedCanonicalFileLocators(agent, agentFilesById, {
omittedRootKeys: AGENT_FILE_INSPECTION_OMITTED_ROOT_KEYS,
onTraversalFailure: input.onTraversalFailure,
messageCount: input.storedMessages?.length ?? 0,
}),
Expand Down
11 changes: 10 additions & 1 deletion packages/api/src/protection/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,7 @@ function omitResolvedFileLocators(
depth = 0,
state?: {
readonly seen: WeakMap<object, unknown>;
readonly omittedRootKeys?: ReadonlySet<string>;
visited: number;
},
): unknown {
Expand Down Expand Up @@ -1098,6 +1099,9 @@ function omitResolvedFileLocators(
};

for (const [key, child] of entries) {
if (depth === 0 && traversal.omittedRootKeys?.has(key)) {
continue;
}
if (resolvedFile != null && key === 'file_id') {
continue;
}
Expand Down Expand Up @@ -1143,14 +1147,19 @@ export function omitResolvedCanonicalFileLocators<T>(
resolvedFilesById: ReadonlyMap<string, CanonicalFileInspectionFile>,
context: {
readonly messageCount?: number;
readonly omittedRootKeys?: ReadonlySet<string>;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor root omissions when no files resolve

When files.pii.uninspectable is block and an agent has no hydrated file with a usable file_id, agentFilesById is empty, so the early return at line 1154 ignores this new omittedRootKeys option. assertInspectableFileInput consequently receives the original agent and still traverses toolDefinitions; a sufficiently deep or large initialized schema rejects an otherwise valid attachment-free turn as content_filter_uninspectable. Apply the root-key omission even for an empty resolved-file map, and retain the fast path only when no omissions were requested.

Useful? React with 馃憤聽/ 馃憥.

readonly onTraversalFailure?: LocatorTraversalReporter;
} = {},
): T {
if (resolvedFilesById.size === 0) {
return input;
}
try {
return omitResolvedFileLocators(input, resolvedFilesById) as T;
return omitResolvedFileLocators(input, resolvedFilesById, 0, {
seen: new WeakMap<object, unknown>(),
omittedRootKeys: context.omittedRootKeys,
visited: 0,
}) as T;
} catch (error) {
if (error instanceof ContentTraversalLimitError && error.diagnostics != null) {
context.onTraversalFailure?.({
Expand Down
Loading