Repository navigation
Conversation
Add an enterprise-gated org policy that restricts routing to providers meeting required certifications/data policies (SOC 2, ISO 27001, GDPR, no prompt training, no prompt logging). The gateway blocks requests to non-compliant providers with a 403 and records a security event. - packages/models: ProviderCompliancePolicy type + isProviderCompliant() - db: organization.providerCompliancePolicy json column + migration - gateway: filter providers by policy, block + logViolation when none qualify - api: org schema/update gate (enterprise, owner/admin) + audit log - ui: new enterprise-gated Compliance settings page + sidebar entry Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an enterprise-only "Provider Compliance Policy" feature end-to-end: a new ChangesProvider Compliance Policy & Multi-Endpoint Enforcement
Web Search Tool Domain Filtering & Anthropic Server-Tool Support
ByteDance Seedance 2.0 Frame Input Support & Video Content Delivery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6514f8db6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| await enforceCompliancePolicy(); |
There was a problem hiding this comment.
Defer compliance checks for auto routing
For model: "auto", this check runs before the auto-routing block resolves a real upstream model/provider, so modelInfo.providers is still the synthetic llmgateway provider from llmgatewayModels. Because that provider's policy metadata has certifications/GDPR set to false, enabling common policies like requireSoc2 or requireGdpr filters the only synthetic provider out and returns 403 before auto-routing can pick a compliant provider such as OpenAI/Anthropic. Skip this pre-auto check or apply compliance while selecting auto candidates instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/routes/organization.ts (2)
102-110:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAllow PATCH to clear the policy back to
null.The response schema and DB column both allow
providerCompliancePolicy: null, but the update schema rejectsnull, so clients cannot restore the “no policy configured” state once a policy object has been saved.Proposed fix
- providerCompliancePolicy: providerCompliancePolicySchema.optional(), + providerCompliancePolicy: providerCompliancePolicySchema.nullable().optional(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/organization.ts` around lines 102 - 110, The updateOrganizationSchema in the organization routes file rejects null values for providerCompliancePolicy field, preventing clients from clearing a previously set policy back to null. Modify the providerCompliancePolicy field in the updateOrganizationSchema object to accept null values in addition to being optional. Use the nullable() method on the providerCompliancePolicySchema to allow explicit null assignments, which will align the schema with the database column and response schema that both support null values.
525-548: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winType
updateDatainstead of widening the new policy write throughany.The new
providerCompliancePolicyassignment is currently unchecked becauseupdateDataisany; a misspelled field or wrong JSON shape would bypass TypeScript.Proposed fix
- const updateData: any = {}; + const updateData: Partial<typeof tables.organization.$inferInsert> = {};As per coding guidelines, “Never use
anyoras anyin TypeScript unless absolutely necessary.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/organization.ts` around lines 525 - 548, The updateData variable is typed as any, which bypasses TypeScript type checking for all properties being assigned to it including providerCompliancePolicy, allowing misspelled fields or incorrect types to pass undetected. Replace the any type annotation on updateData with a properly typed interface or type that includes all the optional properties being assigned (name, billingEmail, billingCompany, billingAddress, billingTaxId, billingNotes, retentionLevel, and providerCompliancePolicy). This will ensure TypeScript validates each assignment at compile time and catches any configuration errors in the update object.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/organization.ts`:
- Around line 614-619: The providerCompliancePolicy field in the organization.ts
file is recording audit changes even when the old and new values are identical.
Fix this by adding a comparison check to the conditional statement that guards
the changes.providerCompliancePolicy assignment. The condition should verify not
only that providerCompliancePolicy is defined, but also that it actually differs
from oldOrg.providerCompliancePolicy before recording it as a change, ensuring
audit logs only contain actual modifications rather than no-op saves.
In `@apps/gateway/src/chat/chat.ts`:
- Around line 2442-2481: The enforceCompliancePolicy function is being called
after the usedProvider has already been selected for auto-routing, which means
non-compliant providers can be picked as the cheapest option before the
compliance check blocks them. Move the compliance filtering to occur before
auto-routing selects a provider so that filtering is applied to the candidate
provider set during pricing and selection. This requires applying the compliance
filtering logic earlier in the flow, specifically before the auto-routing logic
determines which provider to use, rather than checking compliance against an
already-selected usedProvider. Ensure that filterCompliantProviders is called on
the iamFilteredModelProviders and expandedIamFilteredModelProviders before they
are used in provider selection at both locations where auto-routing could pick a
provider.
- Around line 2463-2476: The catch block for the logViolation function call in
the provider compliance check is silently ignoring failures without any logging,
which makes audit gaps invisible and difficult to diagnose. Replace the empty
catch block (currently just containing a comment) with actual error logging. Log
the caught error to ensure that when logViolation fails, the failure is recorded
and visible in audit logs for diagnostic purposes.
In `@apps/ui/src/app/dashboard/`[orgId]/org/compliance/compliance-client.tsx:
- Around line 143-149: The loading condition in compliance-client.tsx checks
both isLoadingTeam and !currentUserRole, causing the spinner to display
indefinitely when currentUserRole remains undefined after useTeamMembers
finishes loading (when the user isn't in the members list). Remove the
!currentUserRole check from the if condition so it only checks isLoadingTeam,
allowing undefined roles to proceed through to the canManage access-denied check
that follows.
- Around line 96-101: The policy state initialized with useState only reads the
selectedOrganization value once on mount, so switching between organizations
leaves the policy state stale and can cause handleSave to persist the wrong
organization's policy. Add a useEffect hook that watches selectedOrganization as
a dependency and updates the policy state whenever the organization context
changes. Additionally, the loading condition at line 143 using isLoadingTeam ||
!currentUserRole doesn't distinguish between a loading state and a failed team
query request. When the team fetch fails, isLoadingTeam becomes false but
currentUserRole remains undefined, showing an indefinite spinner. Add error
state handling to the team query hook to detect fetch failures and display an
error message or retry option instead of the spinner when the request fails.
---
Outside diff comments:
In `@apps/api/src/routes/organization.ts`:
- Around line 102-110: The updateOrganizationSchema in the organization routes
file rejects null values for providerCompliancePolicy field, preventing clients
from clearing a previously set policy back to null. Modify the
providerCompliancePolicy field in the updateOrganizationSchema object to accept
null values in addition to being optional. Use the nullable() method on the
providerCompliancePolicySchema to allow explicit null assignments, which will
align the schema with the database column and response schema that both support
null values.
- Around line 525-548: The updateData variable is typed as any, which bypasses
TypeScript type checking for all properties being assigned to it including
providerCompliancePolicy, allowing misspelled fields or incorrect types to pass
undetected. Replace the any type annotation on updateData with a properly typed
interface or type that includes all the optional properties being assigned
(name, billingEmail, billingCompany, billingAddress, billingTaxId, billingNotes,
retentionLevel, and providerCompliancePolicy). This will ensure TypeScript
validates each assignment at compile time and catches any configuration errors
in the update object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1868a3af-f2f6-4f62-a327-3d8c157eebb0
📒 Files selected for processing (14)
apps/api/src/routes/organization.tsapps/gateway/src/api.spec.tsapps/gateway/src/chat/chat.tsapps/gateway/src/lib/rate-limit.spec.tsapps/ui/src/app/dashboard/[orgId]/org/compliance/compliance-client.tsxapps/ui/src/app/dashboard/[orgId]/org/compliance/contact-sales-card.tsxapps/ui/src/app/dashboard/[orgId]/org/compliance/page.tsxapps/ui/src/components/dashboard/dashboard-sidebar.tsxpackages/db/migrations/1781627146_great_sally_floyd.sqlpackages/db/migrations/meta/1781627146_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.tspackages/models/src/compliance.spec.tspackages/models/src/providers.ts
| try { | ||
| await logViolation( | ||
| project.organizationId, | ||
| { | ||
| ruleId: "provider_compliance", | ||
| ruleName: "Provider compliance policy", | ||
| category: "provider_compliance", | ||
| action: "block", | ||
| }, | ||
| { apiKeyId: apiKey.id, model: requestedModel }, | ||
| ); | ||
| } catch { | ||
| // Silently ignore logging failures | ||
| } |
There was a problem hiding this comment.
Don’t silently drop compliance security-event failures.
A 403 can be returned with no provider_compliance event if logViolation fails here, and the catch leaves no diagnostic. At minimum, log the failure so audit gaps are visible.
Suggested fix
- } catch {
- // Silently ignore logging failures
+ } catch (error) {
+ logger.error("Failed to log provider compliance violation", {
+ error: toError(error),
+ organizationId: project.organizationId,
+ apiKeyId: apiKey.id,
+ model: requestedModel,
+ });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| await logViolation( | |
| project.organizationId, | |
| { | |
| ruleId: "provider_compliance", | |
| ruleName: "Provider compliance policy", | |
| category: "provider_compliance", | |
| action: "block", | |
| }, | |
| { apiKeyId: apiKey.id, model: requestedModel }, | |
| ); | |
| } catch { | |
| // Silently ignore logging failures | |
| } | |
| try { | |
| await logViolation( | |
| project.organizationId, | |
| { | |
| ruleId: "provider_compliance", | |
| ruleName: "Provider compliance policy", | |
| category: "provider_compliance", | |
| action: "block", | |
| }, | |
| { apiKeyId: apiKey.id, model: requestedModel }, | |
| ); | |
| } catch (error) { | |
| logger.error("Failed to log provider compliance violation", { | |
| error: toError(error), | |
| organizationId: project.organizationId, | |
| apiKeyId: apiKey.id, | |
| model: requestedModel, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/chat/chat.ts` around lines 2463 - 2476, The catch block for
the logViolation function call in the provider compliance check is silently
ignoring failures without any logging, which makes audit gaps invisible and
difficult to diagnose. Replace the empty catch block (currently just containing
a comment) with actual error logging. Log the caught error to ensure that when
logViolation fails, the failure is recorded and visible in audit logs for
diagnostic purposes.
- changelog: Provider Compliance Policies entry + hero image (generated with gpt-image-2, LLM Gateway brand mark) - docs: features/compliance.mdx - enterprise-features: add "compliance" feature (powers /enterprise/compliance) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 80%, saving 859.0 KB.
|
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 17%, saving 36.5 KB.
|
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 10.8%, saving 19.3 KB.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72498cc398
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const compliancePolicy = | ||
| organization.plan === "enterprise" && | ||
| organization.providerCompliancePolicy?.enabled | ||
| ? organization.providerCompliancePolicy | ||
| : undefined; |
There was a problem hiding this comment.
Enforce compliance outside chat completions
This policy is only wired into the chat completions router, but other gateway endpoints still send user content directly to providers without any isProviderCompliant check; I checked apps/gateway/src/embeddings/embeddings.ts, apps/gateway/src/speech/speech.ts, apps/gateway/src/moderations/moderations.ts, and apps/gateway/src/videos/videos.ts, and they each load the organization then resolve provider keys independently. For an Enterprise org that enables a policy such as blockPromptLogging, /v1/embeddings or speech/video requests can still reach a non-compliant provider, which contradicts the new guarantee that requests are blocked before data leaves the gateway.
Useful? React with 👍 / 👎.
| if (providerCompliancePolicy !== undefined) { | ||
| updateData.providerCompliancePolicy = providerCompliancePolicy; |
There was a problem hiding this comment.
Invalidate cached organizations on policy updates
Saving the new policy through this route updates the organization with the uncached db client, while the gateway reads organizations through the cached cdb path in findOrganizationById; only cached-client mutations trigger RedisCache.onMutate and SWR invalidation. If an admin enables a stricter policy after the org was cached, the gateway can continue using the old providerCompliancePolicy until the cache expires, allowing non-compliant traffic during that window.
Useful? React with 👍 / 👎.
Compliance reused the Shield icon from Guardrails. Add a dedicated AnimatedBadgeCheck sidebar icon and a "badge-check" enterprise-feature icon (with docs frontmatter) so Compliance is visually distinct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/ui/src/app/enterprise/[slug]/page.tsx (1)
34-42: ⚡ Quick winCentralize enterprise icon mapping to avoid cross-file drift.
iconMapis duplicated here and inapps/ui/src/components/enterprise/capabilities.tsx, which already required parallel edits for"badge-check". Extract a shared mapping (e.g.,apps/ui/src/lib/enterprise-icons.ts) and import it in both places.As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Apply DRY (Don't Repeat Yourself) principles for code reuse."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ui/src/app/enterprise/`[slug]/page.tsx around lines 34 - 42, The iconMap constant is duplicated across multiple files, violating DRY principles. Create a new shared file (e.g., enterprise-icons.ts in the lib directory) that exports the iconMap object containing all icon mappings (shield-check, badge-check, git-branch, audit, bell, lock, paintbrush). Then replace the iconMap definition in the current file and in apps/ui/src/components/enterprise/capabilities.tsx with imports from the new shared file, ensuring both locations reference the same centralized mapping.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/ui/src/app/enterprise/`[slug]/page.tsx:
- Around line 34-42: The iconMap constant is duplicated across multiple files,
violating DRY principles. Create a new shared file (e.g., enterprise-icons.ts in
the lib directory) that exports the iconMap object containing all icon mappings
(shield-check, badge-check, git-branch, audit, bell, lock, paintbrush). Then
replace the iconMap definition in the current file and in
apps/ui/src/components/enterprise/capabilities.tsx with imports from the new
shared file, ensuring both locations reference the same centralized mapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4d354397-e341-4ad7-bac2-51971aafbb94
⛔ Files ignored due to path filters (1)
apps/ui/public/changelog/provider-compliance-policies.pngis excluded by!**/*.png
📒 Files selected for processing (7)
apps/docs/content/features/compliance.mdxapps/ui/src/app/enterprise/[slug]/page.tsxapps/ui/src/components/dashboard/animated-nav-icons.tsxapps/ui/src/components/dashboard/dashboard-sidebar.tsxapps/ui/src/components/enterprise/capabilities.tsxapps/ui/src/content/changelog/2026-06-16-provider-compliance-policies.mdapps/ui/src/lib/enterprise-features.ts
✅ Files skipped from review due to trivial changes (1)
- apps/ui/src/content/changelog/2026-06-16-provider-compliance-policies.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/components/dashboard/dashboard-sidebar.tsx
Provider Impact now lists both allowed and blocked providers as larger logo chips (color-coded with check/ban icons) for at-a-glance review, and hides the internal llmgateway and custom providers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Summary Two related changes, both surfaced while testing ByteDance Seedance 2.0 video generation in the playground. ### 1. First/last frame inputs for Seedance 2.0 ByteDance Seedance 2.0 supports first/last frame conditioning for image-to-video, but the gateway and playground didn't expose it: - The playground hid the **First/Last frame** pickers behind `supportsVideoFrameInput()`, which only returned `true` for Veo / MiniMax / Grok models. - The gateway's `createBytedanceVideoJob()` only forwarded the **first** frame — a `last_frame` was silently dropped, and frame inputs were rejected for ByteDance by the constraint checker. Changes: - **Gateway** (`videos.ts`): forward `last_frame` as a `role: "last_frame"` content entry; allow `inputMode === "frames"` for Seedance 2.0 mappings (non-2.0 ByteDance models still rejected with a clear message); renamed `isBytedanceReferenceModel` → `isBytedanceSeedance2Model`. - **Playground** (`video-gen.ts`): `supportsVideoFrameInput()` returns `true` for Seedance 2.0. - **Docs** (`video-generation.mdx`): new **First/last frame inputs** section with a provider table (Seedance 2.0 called out), rules, and example. - **Tests**: ByteDance video mock handler + forwarding/rejection tests; playground capability test. ### 2. Fix "Video unavailable" for cross-project video playback After generating a video, the playground could show **"Video unavailable"** even though the job completed successfully with a valid content URL. Root cause: the playground polls video **status** via the session/org-authorized API app, but loaded the **content** through the project-scoped gateway API key. When the active playground key's project differed from the project that owns the job (e.g. after an org/project switch, or a rotated auto-generated key), the gateway returned `404 "Video not found"`. Verified against a real completed Seedance 2.0 job: the gateway returned the full video to the owning key (200) but 404 to a different project's key — exactly the playground's symptom. Changes: - **API** (`routes/video.ts`): the video status route now returns a keyless signed content URL (`buildSignedGatewayVideoLogContentUrl`), scoped by the same org-access check it already performs. - **Playground** (`[videoId]/content/route.ts`): the content proxy resolves that signed URL via the session-authorized API instead of the project-scoped gateway key. Playback now works for any video the user can access, and stored history URLs stay durable (each load re-resolves a fresh signed URL). ## Verification - `pnpm format` ✅ · `pnpm build` ✅ · docs build ✅ - Gateway video spec ✅ 30/30 · playground video-gen spec ✅ 13/13 - End-to-end against a real completed job: API-route-produced signed URL streams the full 8.9 MB video keyless (HTTP 200); confirmed the gateway 404s for a wrong-project key (the original bug) and 200s for the owning key. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added support for Seedance 2.0 `image`/`last_frame` inputs using the Bytedance provider. * Video job status responses for completed videos can now include `content` (signed playback URLs). * **Bug Fixes** * Enforced frame/reference validation rules and improved rejection messaging for incompatible model/provider combinations. * **Tests** * Added/extended test coverage for Seedance 2.0 Bytedance frame requests, compatibility detection, and OpenAI mock task generation. * **Documentation** * Updated `POST /v1/videos` supported fields and added first/last frame interpolation guidance and constraints. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Problem
The native Anthropic `/v1/messages` endpoint rejected valid requests
that included Anthropic **server-side tools** (e.g.
`web_search_20250305`). Sending Anthropic's documented web search
request:
```json
{
"model": "claude-sonnet-4-6",
"max_tokens": 4096,
"messages": [{ "role": "user", "content": "Search the web ..." }],
"tools": [{ "type": "web_search_20250305", "name": "web_search", "max_uses": 3 }]
}
```
returned a `ZodError`:
```json
{"success":false,"error":{"issues":[
{"code":"invalid_type","path":["tools",0,"description"],"message":"Required"},
{"code":"invalid_type","path":["tools",0,"input_schema"],"message":"Required"}
],"name":"ZodError"}}
```
The same request works when sent directly to `api.anthropic.com`.
## Root cause
The endpoint's tool schema (`anthropicToolSchema`) required
`description` **and** `input_schema` on **every** tool. Server-side
tools carry a versioned `type` and have neither field, so validation
failed before the request ever reached a provider.
## Fix
- Split the tool schema into a union: standard **custom tools** (`name`
+ `input_schema`) and Anthropic **server-side tools** (`type` + `name` +
optional config like `max_uses`, `user_location`, `allowed_domains`).
- In the Anthropic → OpenAI translation, map `web_search*` server tools
to the internal `web_search` tool that the chat completions endpoint
already forwards to Anthropic as `web_search_20250305` (preserving
`max_uses`/`user_location`).
- Unsupported server tools are dropped with a warning rather than
rejecting the whole request.
## Tests
Added two unit tests in `apps/gateway/src/api.spec.ts`:
- A `web_search_20250305` server tool passes validation and is forwarded
to the Anthropic provider as a native `web_search_20250305` tool.
- A malformed custom tool (missing `input_schema`) is still rejected
with `400`.
`pnpm build`, `pnpm format`, and the new tests all pass.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
## Release Notes
* **New Features**
* Added support for Anthropic native server-side tools (such as web
search) alongside custom tools
* Enhanced tool configuration with additional parameters including
allowed domains, blocked domains, and user location settings
* **Tests**
* Added comprehensive test coverage for server-side tool handling and
custom tool validation
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the changelog hero with the new on-brand graphic, reframed to 16:9 (1792x1024) via gpt-image-2 image edit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 88%, saving 1.5 MB.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66d9f3156d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| where: { | ||
| requestId: { eq: job.requestId }, | ||
| }, |
There was a problem hiding this comment.
Scope signed content lookup to the job
For completed jobs this query finds the first log with the same requestId across the whole log table, then signs that log's content URL. Video requestId is taken from caller-supplied x-request-id and is only indexed, not unique, so if two jobs share a request id maliciously or accidentally, a user authorized for their own job can receive a signed URL for another project/org's log. Include at least job.projectId/job.organizationId in the log lookup before signing.
Useful? React with 👍 / 👎.
| const anthropicServerToolSchema = z.object({ | ||
| type: z.string(), |
There was a problem hiding this comment.
Reject custom tools before server-tool parsing
Because this server-tool schema accepts any string type, an Anthropic custom tool such as { type: "custom", name: "..." } that is missing or misspells input_schema no longer fails validation: it matches this branch, then the mapper treats custom as an unsupported server tool and silently drops it. That turns malformed tool requests into normal model calls without tools instead of returning the expected 400, so constrain this schema to real server-tool types or explicitly exclude custom.
Useful? React with 👍 / 👎.
- gateway: fix auto-routing false-positive — skip the pre-resolution enforcement for model:"auto" (it ran against the synthetic llmgateway provider) and filter auto candidates by policy during selection - gateway: enforce compliance on embeddings, speech, moderations and videos too (shared lib/compliance.ts), so the "blocked before data leaves the gateway" guarantee holds across endpoints - gateway: log (don't silently drop) compliance security-event failures - api: allow clearing the policy to null; only audit real policy changes - ui: don't hang the spinner on undefined role; reset policy when the selected organization changes Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the review feedback (commit pushed): Fixed
Not changed (with reasons)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b4abeab32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const compliantProviders = filterCompliantProviders( | ||
| modelInfo.providers as ProviderModelMapping[], | ||
| videoCompliancePolicy, | ||
| ); | ||
| if (compliantProviders.length === 0) { |
There was a problem hiding this comment.
Block pinned non-compliant video providers
This only blocks when all providers for the video model are filtered out. If the request is pinned to a non-compliant provider but the model has another compliant provider (for example avalanche/veo-3.1-generate-preview under a SOC 2 policy, where google-vertex remains in compliantProviders), resolveVideoExecution later sees no mappings for the requested provider and returns a generic 400 without recording the compliance block. Check requestedProvider against the policy here, like the chat path does, so pinned video requests get the expected policy 403/security event.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
apps/gateway/src/api.spec.ts (1)
241-284: ⚡ Quick winAdd persistence assertion for the embeddings compliance violation path.
This test validates the 403 response, but it doesn’t verify that a
provider_complianceguardrail event is persisted for embeddings (the same guarantee already asserted in the chat compliance test).🧪 Suggested assertion addition
expect(res.status).toBe(403); const json = await res.json(); expect(json.error.message).toContain("provider compliance policy"); + + const violations = await db.query.guardrailViolation.findMany({ + where: { organizationId: { eq: "org-id" } }, + }); + expect(violations.some((v) => v.category === "provider_compliance")).toBe( + true, + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/api.spec.ts` around lines 241 - 284, After validating the 403 status and error message in the "/v1/embeddings is blocked by the compliance policy too" test, add an assertion that verifies a provider_compliance guardrail event was persisted to the database. Query the guardrail events table and assert that an event with type "provider_compliance" exists for this request, following the same pattern already established in the chat compliance policy test to ensure consistency in compliance violation tracking across all endpoint types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/video.ts`:
- Around line 109-116: The log lookup in the db.query.log.findFirst call at
apps/api/src/routes/video.ts (lines 109-116) only filters by requestId, which
can cause ID collisions across different projects or organizations. To fix this
security issue, add projectId or organizationId to the where clause predicate
alongside the requestId filter. Retrieve the appropriate scoping ID from the job
object or request context and include it in the findFirst query to ensure the
log record belongs to the correct project or organization.
In `@apps/gateway/src/anthropic/anthropic.ts`:
- Around line 113-114: The schema currently allows both `allowed_domains` and
`blocked_domains` fields to be optionally set simultaneously, which creates
ambiguous behavior for server-tool routing. Add mutual-exclusion validation to
the schema to ensure that only one of these two fields can be provided at a
time. Use Zod's .refine() or .superRefine() method on the parent schema object
to validate that either `allowed_domains` is set XOR `blocked_domains` is set,
but not both. The validation should reject requests where both fields are
populated with values.
In `@apps/gateway/src/api.spec.ts`:
- Line 5228: The variable `capturedBody` declared on line 5228 uses the bare
`any` type which violates TypeScript guidelines. Replace the `any` type
annotation with an explicit type structure that describes the expected request
body. Examine how `capturedBody` is used throughout the test to determine the
appropriate properties and their types (such as string, number, object, array,
etc.), then either define an inline type literal (using the type keyword or as a
const assertion) or use an existing interface that represents the request body
structure.
In `@apps/gateway/src/chat/chat.ts`:
- Around line 2671-2679: The code applies compliance filtering via
`applyCompliancePolicy` on `cachedFilteredProviders` but does not check whether
this filtering removes all candidates. When compliance filtering results in zero
compliant providers, the code falls through to generic error messages or
fallback logic instead of returning a proper 403 compliance error. Add a check
after the `applyCompliancePolicy` call to detect when
`complianceFilteredProviders` is empty, and immediately fail closed by returning
a 403 response with `provider_compliance` logged as the error reason, before
reaching any fallback paths or generic error handling. Apply this same check at
both affected sites mentioned in the comment.
- Around line 2479-2484: The checkOpenAIContentFilter function calls OpenAI's
moderation API without first verifying that OpenAI complies with the
organization's active compliance policy, creating a potential data leakage
vulnerability. Before invoking checkOpenAIContentFilter (at startLine 2479-2484
and also at lines 3116-3137), add a compliance guard that checks whether OpenAI
is permitted under the current policy. The guard should fail-closed (deny the
operation) if OpenAI does not comply with the active policy, preventing any
prompt data from being sent to OpenAI when the organization has blocked it. This
check should be similar in structure to the existing enforceCompliancePolicy
pattern but specifically targeting the OpenAI provider before the content filter
call.
In `@apps/gateway/src/chat/schemas/completions.ts`:
- Around line 247-248: The schema allows both allowed_domains and
blocked_domains to be provided simultaneously, which represents conflicting
intent. Add validation logic to the parent object containing these two fields
(using Zod's refine or superRefine method) to reject requests where both
allowed_domains and blocked_domains are provided at the same time. This
validation should fail fast during request validation with a clear error message
indicating that only one of these domain filters can be used per request.
In `@apps/gateway/src/videos/videos.ts`:
- Around line 3902-3917: The compliance check block currently only validates
that at least one compliant provider exists after filtering, but does not check
if a specifically requested provider is compliant. This allows a pinned
non-compliant provider to bypass the 403 error and security logging. Add an
explicit compliance check for the requestedProvider (if one is specified) before
the existing videoCompliancePolicy filtering logic. If the requestedProvider is
explicitly non-compliant, immediately call logComplianceBlock and throw the 403
HTTPException with the compliance block message, using the same pattern as the
existing code but checking the specific requested provider against the
videoCompliancePolicy before attempting to filter all providers.
In `@packages/models/src/types.ts`:
- Around line 542-551: The WebSearchTool type currently allows both
allowed_domains and blocked_domains to be optionally defined, but the
documentation states they are mutually exclusive. Update the type definition to
enforce this constraint using TypeScript's discriminated union pattern. Create
separate type variations that represent each mutually exclusive case: one
allowing only allowed_domains (with blocked_domains omitted or never), and
another allowing only blocked_domains (with allowed_domains omitted or never).
Use a union of these types for the WebSearchTool definition to ensure the type
contract prevents both properties from being set simultaneously while still
allowing neither to be set.
---
Nitpick comments:
In `@apps/gateway/src/api.spec.ts`:
- Around line 241-284: After validating the 403 status and error message in the
"/v1/embeddings is blocked by the compliance policy too" test, add an assertion
that verifies a provider_compliance guardrail event was persisted to the
database. Query the guardrail events table and assert that an event with type
"provider_compliance" exists for this request, following the same pattern
already established in the chat compliance policy test to ensure consistency in
compliance violation tracking across all endpoint types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 66a445b1-159f-4aec-9cf1-b63edac998db
⛔ Files ignored due to path filters (1)
apps/ui/public/changelog/provider-compliance-policies.pngis excluded by!**/*.png
📒 Files selected for processing (21)
apps/api/src/routes/organization.tsapps/api/src/routes/video.tsapps/docs/content/features/video-generation.mdxapps/gateway/src/anthropic/anthropic.tsapps/gateway/src/api.spec.tsapps/gateway/src/chat/chat.tsapps/gateway/src/chat/schemas/completions.tsapps/gateway/src/embeddings/embeddings.tsapps/gateway/src/lib/compliance.tsapps/gateway/src/moderations/moderations.tsapps/gateway/src/speech/speech.tsapps/gateway/src/test-utils/mock-openai-server.tsapps/gateway/src/videos/videos.spec.tsapps/gateway/src/videos/videos.tsapps/playground/src/app/api/video/[videoId]/content/route.tsapps/playground/src/lib/video-gen.spec.tsapps/playground/src/lib/video-gen.tsapps/ui/src/app/dashboard/[orgId]/org/compliance/compliance-client.tsxapps/ui/src/content/changelog/2026-06-16-provider-compliance-policies.mdpackages/actions/src/prepare-request-body.tspackages/models/src/types.ts
✅ Files skipped from review due to trivial changes (1)
- apps/ui/src/content/changelog/2026-06-16-provider-compliance-policies.md
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/routes/organization.ts
- apps/ui/src/app/dashboard/[orgId]/org/compliance/compliance-client.tsx
| const log = await db.query.log.findFirst({ | ||
| where: { | ||
| requestId: { eq: job.requestId }, | ||
| }, | ||
| columns: { | ||
| id: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Scope the log lookup to the same project (or organization), not only requestId.
findFirst on requestId alone can resolve the wrong log record if IDs collide/reuse. That can generate a signed URL for unrelated content. Include projectId (or organizationId) in the lookup predicate.
Suggested fix
const log = await db.query.log.findFirst({
where: {
requestId: { eq: job.requestId },
+ projectId: { eq: job.projectId },
},
columns: {
id: true,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const log = await db.query.log.findFirst({ | |
| where: { | |
| requestId: { eq: job.requestId }, | |
| }, | |
| columns: { | |
| id: true, | |
| }, | |
| }); | |
| const log = await db.query.log.findFirst({ | |
| where: { | |
| requestId: { eq: job.requestId }, | |
| projectId: { eq: job.projectId }, | |
| }, | |
| columns: { | |
| id: true, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/routes/video.ts` around lines 109 - 116, The log lookup in the
db.query.log.findFirst call at apps/api/src/routes/video.ts (lines 109-116) only
filters by requestId, which can cause ID collisions across different projects or
organizations. To fix this security issue, add projectId or organizationId to
the where clause predicate alongside the requestId filter. Retrieve the
appropriate scoping ID from the job object or request context and include it in
the findFirst query to ensure the log record belongs to the correct project or
organization.
| allowed_domains: z.array(z.string()).optional(), | ||
| blocked_domains: z.array(z.string()).optional(), |
There was a problem hiding this comment.
Add mutual-exclusion validation for Anthropic server tool domain filters
Line 113 and Line 114 currently permit both allowed_domains and blocked_domains. This should be rejected at schema validation to avoid ambiguous server-tool routing behavior.
Suggested fix
-const anthropicServerToolSchema = z.object({
- type: z.string(),
- name: z.string(),
- max_uses: z.number().optional(),
- allowed_domains: z.array(z.string()).optional(),
- blocked_domains: z.array(z.string()).optional(),
- user_location: z
- .object({
- type: z.literal("approximate").optional(),
- city: z.string().optional(),
- region: z.string().optional(),
- country: z.string().optional(),
- timezone: z.string().optional(),
- })
- .optional(),
- cache_control: z
- .object({
- type: z.enum(["ephemeral"]),
- ttl: z.enum(["5m", "1h"]).optional(),
- })
- .nullish(),
-});
+const anthropicServerToolSchema = z
+ .object({
+ type: z.string(),
+ name: z.string(),
+ max_uses: z.number().optional(),
+ allowed_domains: z.array(z.string()).optional(),
+ blocked_domains: z.array(z.string()).optional(),
+ user_location: z
+ .object({
+ type: z.literal("approximate").optional(),
+ city: z.string().optional(),
+ region: z.string().optional(),
+ country: z.string().optional(),
+ timezone: z.string().optional(),
+ })
+ .optional(),
+ cache_control: z
+ .object({
+ type: z.enum(["ephemeral"]),
+ ttl: z.enum(["5m", "1h"]).optional(),
+ })
+ .nullish(),
+ })
+ .superRefine((tool, ctx) => {
+ if (tool.allowed_domains && tool.blocked_domains) {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ message:
+ "allowed_domains and blocked_domains are mutually exclusive",
+ path: ["allowed_domains"],
+ });
+ }
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| allowed_domains: z.array(z.string()).optional(), | |
| blocked_domains: z.array(z.string()).optional(), | |
| const anthropicServerToolSchema = z | |
| .object({ | |
| type: z.string(), | |
| name: z.string(), | |
| max_uses: z.number().optional(), | |
| allowed_domains: z.array(z.string()).optional(), | |
| blocked_domains: z.array(z.string()).optional(), | |
| user_location: z | |
| .object({ | |
| type: z.literal("approximate").optional(), | |
| city: z.string().optional(), | |
| region: z.string().optional(), | |
| country: z.string().optional(), | |
| timezone: z.string().optional(), | |
| }) | |
| .optional(), | |
| cache_control: z | |
| .object({ | |
| type: z.enum(["ephemeral"]), | |
| ttl: z.enum(["5m", "1h"]).optional(), | |
| }) | |
| .nullish(), | |
| }) | |
| .superRefine((tool, ctx) => { | |
| if (tool.allowed_domains && tool.blocked_domains) { | |
| ctx.addIssue({ | |
| code: z.ZodIssueCode.custom, | |
| message: | |
| "allowed_domains and blocked_domains are mutually exclusive", | |
| path: ["allowed_domains"], | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/anthropic/anthropic.ts` around lines 113 - 114, The schema
currently allows both `allowed_domains` and `blocked_domains` fields to be
optionally set simultaneously, which creates ambiguous behavior for server-tool
routing. Add mutual-exclusion validation to the schema to ensure that only one
of these two fields can be provided at a time. Use Zod's .refine() or
.superRefine() method on the parent schema object to validate that either
`allowed_domains` is set XOR `blocked_domains` is set, but not both. The
validation should reject requests where both fields are populated with values.
| baseUrl: mockServerUrl, | ||
| }); | ||
|
|
||
| let capturedBody: any; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no forbidden `any` usages remain in this test file.
rg -nP '\bany\b|as any\b' apps/gateway/src/api.spec.tsRepository: theopenco/llmgateway
Length of output: 622
Replace any type with explicit type structure for captured request body.
Line 5228 uses a bare any type annotation, which violates the TypeScript guideline. Replace with an explicit type that captures the expected structure of the request body:
Suggested fix
- let capturedBody: any;
+ let capturedBody: { tools?: Array<{ type?: string; [key: string]: unknown }> } | undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let capturedBody: any; | |
| let capturedBody: { tools?: Array<{ type?: string; [key: string]: unknown }> } | undefined; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/api.spec.ts` at line 5228, The variable `capturedBody`
declared on line 5228 uses the bare `any` type which violates TypeScript
guidelines. Replace the `any` type annotation with an explicit type structure
that describes the expected request body. Examine how `capturedBody` is used
throughout the test to determine the appropriate properties and their types
(such as string, number, object, array, etc.), then either define an inline type
literal (using the type keyword or as a const assertion) or use an existing
interface that represents the request body structure.
Source: Coding guidelines
| allowed_domains: z.array(z.string()).optional(), | ||
| blocked_domains: z.array(z.string()).optional(), |
There was a problem hiding this comment.
Reject conflicting domain filters at request validation
Line 247 and Line 248 allow both domain lists simultaneously. The API should fail fast when both are provided instead of accepting conflicting intent.
Suggested fix
- z.object({
+ z
+ .object({
type: z.literal("web_search"),
user_location: z
.object({
city: z.string().optional(),
region: z.string().optional(),
country: z.string().optional(),
timezone: z.string().optional(),
})
.optional(),
search_context_size: z.enum(["low", "medium", "high"]).optional(),
max_uses: z.number().optional(),
allowed_domains: z.array(z.string()).optional(),
blocked_domains: z.array(z.string()).optional(),
- }),
+ })
+ .superRefine((tool, ctx) => {
+ if (tool.allowed_domains && tool.blocked_domains) {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ message:
+ "allowed_domains and blocked_domains are mutually exclusive",
+ path: ["allowed_domains"],
+ });
+ }
+ }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| allowed_domains: z.array(z.string()).optional(), | |
| blocked_domains: z.array(z.string()).optional(), | |
| z | |
| .object({ | |
| type: z.literal("web_search"), | |
| user_location: z | |
| .object({ | |
| city: z.string().optional(), | |
| region: z.string().optional(), | |
| country: z.string().optional(), | |
| timezone: z.string().optional(), | |
| }) | |
| .optional(), | |
| search_context_size: z.enum(["low", "medium", "high"]).optional(), | |
| max_uses: z.number().optional(), | |
| allowed_domains: z.array(z.string()).optional(), | |
| blocked_domains: z.array(z.string()).optional(), | |
| }) | |
| .superRefine((tool, ctx) => { | |
| if (tool.allowed_domains && tool.blocked_domains) { | |
| ctx.addIssue({ | |
| code: z.ZodIssueCode.custom, | |
| message: | |
| "allowed_domains and blocked_domains are mutually exclusive", | |
| path: ["allowed_domains"], | |
| }); | |
| } | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/chat/schemas/completions.ts` around lines 247 - 248, The
schema allows both allowed_domains and blocked_domains to be provided
simultaneously, which represents conflicting intent. Add validation logic to the
parent object containing these two fields (using Zod's refine or superRefine
method) to reject requests where both allowed_domains and blocked_domains are
provided at the same time. This validation should fail fast during request
validation with a clear error message indicating that only one of these domain
filters can be used per request.
| /** | ||
| * Restrict search results to these domains (Anthropic). Mutually exclusive | ||
| * with blocked_domains. | ||
| */ | ||
| allowed_domains?: string[]; | ||
| /** | ||
| * Exclude these domains from search results (Anthropic). Mutually exclusive | ||
| * with allowed_domains. | ||
| */ | ||
| blocked_domains?: string[]; |
There was a problem hiding this comment.
Enforce domain filter exclusivity in the type contract
On Line 542 and Line 548, exclusivity is documented but not encoded. WebSearchTool currently allows both allowed_domains and blocked_domains, which creates ambiguous behavior downstream.
Suggested fix
export interface WebSearchTool {
type: "web_search";
user_location?: {
type: "approximate";
city?: string;
region?: string;
country?: string;
timezone?: string;
};
search_context_size?: "low" | "medium" | "high";
max_uses?: number;
- allowed_domains?: string[];
- blocked_domains?: string[];
+ (
+ | { allowed_domains: string[]; blocked_domains?: never }
+ | { blocked_domains: string[]; allowed_domains?: never }
+ | { allowed_domains?: undefined; blocked_domains?: undefined }
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Restrict search results to these domains (Anthropic). Mutually exclusive | |
| * with blocked_domains. | |
| */ | |
| allowed_domains?: string[]; | |
| /** | |
| * Exclude these domains from search results (Anthropic). Mutually exclusive | |
| * with allowed_domains. | |
| */ | |
| blocked_domains?: string[]; | |
| export type WebSearchTool = { | |
| type: "web_search"; | |
| user_location?: { | |
| type: "approximate"; | |
| city?: string; | |
| region?: string; | |
| country?: string; | |
| timezone?: string; | |
| }; | |
| search_context_size?: "low" | "medium" | "high"; | |
| max_uses?: number; | |
| } & ( | |
| | { allowed_domains: string[]; blocked_domains?: never } | |
| | { blocked_domains: string[]; allowed_domains?: never } | |
| | { allowed_domains?: undefined; blocked_domains?: undefined } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/models/src/types.ts` around lines 542 - 551, The WebSearchTool type
currently allows both allowed_domains and blocked_domains to be optionally
defined, but the documentation states they are mutually exclusive. Update the
type definition to enforce this constraint using TypeScript's discriminated
union pattern. Create separate type variations that represent each mutually
exclusive case: one allowing only allowed_domains (with blocked_domains omitted
or never), and another allowing only blocked_domains (with allowed_domains
omitted or never). Use a union of these types for the WebSearchTool definition
to ensure the type contract prevents both properties from being set
simultaneously while still allowing neither to be set.
Address compliance-related review feedback: - chat: skip the OpenAI content-filter call when the policy disallows OpenAI, so prompts never reach a non-compliant moderation endpoint - chat: when auto-routing has compliant providers removed for every candidate, fail closed with the policy 403 + security event instead of the generic errors / hardcoded fallback - videos: block a pinned non-compliant provider explicitly, even when the model has other compliant providers (Unrelated review items — video signed-URL scoping, anthropic tool schema, web_search domain filters — are out of scope for this PR.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Second pass — addressed the compliance-related items from the latest review rounds:
Intentionally not changed (not related to the compliance feature): Gateway build green; the 3 compliance integration tests + 6 model unit tests pass. |
💡 Codex ReviewFor enterprise orgs with a policy such as ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/gateway/src/chat/chat.ts (1)
1539-1546:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
webSearchToolin the cache key before enabling domain-filtered search.
allowed_domains/blocked_domainsnow affect the upstream request, but the cache payload omitswebSearchToolafter the tool is spliced out oftools. Two requests with different domain restrictions can therefore share a cached answer, bypassing the caller’s search constraints.🛡️ Proposed fix
const cachePayload = { provider: usedProvider, model: usedInternalModel, messages, @@ n, service_tier, + webSearchTool: webSearchTool + ? { + type: webSearchTool.type, + user_location: webSearchTool.user_location ?? null, + search_context_size: webSearchTool.search_context_size ?? null, + max_uses: webSearchTool.max_uses ?? null, + allowed_domains: webSearchTool.allowed_domains ?? null, + blocked_domains: webSearchTool.blocked_domains ?? null, + } + : null, };Also applies to: 4850-4866
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/chat/chat.ts` around lines 1539 - 1546, The webSearchTool object is being created with allowed_domains and blocked_domains that affect the upstream request, but this tool is not being included in the cache key before it is removed from the tools array. This means two requests with different domain restrictions could incorrectly share a cached answer, bypassing the caller's search constraints. Include the webSearchTool object (with its allowed_domains and blocked_domains properties) in the cache key calculation to ensure that requests with different domain restrictions are cached separately. Note that this issue also occurs at the other location mentioned in the comment (lines 4850-4866), and the same fix should be applied there.
♻️ Duplicate comments (1)
apps/gateway/src/chat/chat.ts (1)
3153-3162:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed instead of silently disabling the OpenAI content filter.
When
contentFilterMethod === "openai"and OpenAI is non-compliant, this skips moderation and lets the request continue unfiltered. ForcontentFilterMode === "enabled", reject before routing or use a compliant moderation provider.🛡️ Proposed fail-closed guard
const openAiContentFilterAllowed = !compliancePolicy || isProviderIdCompliant("openai", compliancePolicy); + if ( + shouldApplyGatewayContentFilter && + contentFilterMethod === "openai" && + !openAiContentFilterAllowed + ) { + await logComplianceBlock(project.organizationId, { + apiKeyId: apiKey.id, + model: requestedModel, + }); + throw new HTTPException(403, { + message: complianceBlockMessage(modelInfo.id), + }); + } const openAIContentFilterResult = shouldApplyGatewayContentFilter && contentFilterMethod === "openai" &&🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/chat/chat.ts` around lines 3153 - 3162, The code currently skips the OpenAI content filter when it is non-compliant without rejecting the request, which violates the fail-closed principle for content filtering. Add a guard check after determining openAiContentFilterAllowed that rejects the request if contentFilterMode is "enabled" and contentFilterMethod is "openai" but openAiContentFilterAllowed is false. This guard should prevent the request from proceeding unfiltered when compliance policy disallows OpenAI moderation while filtering is mandatory.
🧹 Nitpick comments (1)
apps/gateway/src/chat/chat.ts (1)
1534-1538: Remove explicitanycasts from web-search tool extraction.The
toolsunion can be narrowed by type discriminator without bypassing type safety.♻️ Proposed refactor
const webSearchToolIndex = tools.findIndex( - (tool: any) => tool.type === "web_search", + (tool) => tool.type === "web_search", ); if (webSearchToolIndex !== -1) { - // Cast to any to access properties since the schema allows both function and web_search tools - const foundTool = tools[webSearchToolIndex] as any; - webSearchTool = { - type: "web_search", - user_location: foundTool.user_location, - search_context_size: foundTool.search_context_size, - max_uses: foundTool.max_uses, - allowed_domains: foundTool.allowed_domains, - blocked_domains: foundTool.blocked_domains, - }; - // Remove the web_search tool from the tools array so it's not sent as a regular tool - tools.splice(webSearchToolIndex, 1); + const foundTool = tools[webSearchToolIndex]; + if (foundTool?.type === "web_search") { + webSearchTool = { + type: "web_search", + user_location: foundTool.user_location, + search_context_size: foundTool.search_context_size, + max_uses: foundTool.max_uses, + allowed_domains: foundTool.allowed_domains, + blocked_domains: foundTool.blocked_domains, + }; + tools.splice(webSearchToolIndex, 1); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/chat/chat.ts` around lines 1534 - 1538, The code in the web-search tool extraction block uses an explicit `as any` cast to bypass type safety when accessing properties of the found tool. Instead of casting to any, use TypeScript's type narrowing capabilities based on the type discriminator check (the `type === "web_search"` condition that precedes this code). After confirming that webSearchToolIndex is not -1, you can safely access the tool's properties without the any cast by leveraging the fact that the tools union can be discriminated by the type property. Remove the `as any` cast from the foundTool assignment and let TypeScript automatically narrow the type based on the preceding type check.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 1539-1546: The webSearchTool object is being created with
allowed_domains and blocked_domains that affect the upstream request, but this
tool is not being included in the cache key before it is removed from the tools
array. This means two requests with different domain restrictions could
incorrectly share a cached answer, bypassing the caller's search constraints.
Include the webSearchTool object (with its allowed_domains and blocked_domains
properties) in the cache key calculation to ensure that requests with different
domain restrictions are cached separately. Note that this issue also occurs at
the other location mentioned in the comment (lines 4850-4866), and the same fix
should be applied there.
---
Duplicate comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 3153-3162: The code currently skips the OpenAI content filter when
it is non-compliant without rejecting the request, which violates the
fail-closed principle for content filtering. Add a guard check after determining
openAiContentFilterAllowed that rejects the request if contentFilterMode is
"enabled" and contentFilterMethod is "openai" but openAiContentFilterAllowed is
false. This guard should prevent the request from proceeding unfiltered when
compliance policy disallows OpenAI moderation while filtering is mandatory.
---
Nitpick comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 1534-1538: The code in the web-search tool extraction block uses
an explicit `as any` cast to bypass type safety when accessing properties of the
found tool. Instead of casting to any, use TypeScript's type narrowing
capabilities based on the type discriminator check (the `type === "web_search"`
condition that precedes this code). After confirming that webSearchToolIndex is
not -1, you can safely access the tool's properties without the any cast by
leveraging the fact that the tools union can be discriminated by the type
property. Remove the `as any` cast from the foundTool assignment and let
TypeScript automatically narrow the type based on the preceding type check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5392f6ec-4518-408d-adc2-eb7faee13d4e
📒 Files selected for processing (2)
apps/gateway/src/chat/chat.tsapps/gateway/src/videos/videos.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/gateway/src/videos/videos.ts
Summary
Adds an enterprise-gated provider compliance policy so organizations can guarantee their LLM traffic only reaches providers meeting specific regulatory requirements (the insurance/SOC 2 use case). Requests routed to a non-compliant provider are blocked with a 403 before any data leaves the gateway, and the block is recorded as a security event.
Provider compliance metadata already exists on every provider (
ProviderDataPolicy:soc2,iso27001,gdpr,apiTraining,promptLogging) — this builds the policy + enforcement + UI on top of it.What's included
packages/models—ProviderCompliancePolicytype +isProviderCompliant()helper (fail-closed: a provider passes only if itsdataPolicyexplicitly satisfies each active requirement). Requirements: SOC 2, ISO 27001, SOC 2 or ISO 27001, GDPR, no API training, no prompt logging.packages/db—organization.providerCompliancePolicyjson column (+ migration).apps/gateway— filters candidate providers by the org policy and blocks (403 +logViolation) when none qualify or a pinned provider is non-compliant. Applied on both the direct and auto-routing paths.apps/api— org schema/update support, gated to enterprise plan + owner/admin, with an audit-log entry.apps/ui— new enterprise-gated Compliance settings page (sidebar entry next to Guardrails) with per-requirement toggles and a live "blocked providers" preview. Non-enterprise orgs see a Contact Sales card.docs/features/compliance.mdx, and acomplianceentry inenterprise-features.ts(powers/enterprise/compliance).Enforcement behavior
When no available provider for the requested model meets the policy — or a pinned provider is non-compliant — the gateway returns a 403 explaining the policy and to contact the LLMGateway admin, and writes a
provider_compliancesecurity event (visible under Security Events).Testing
pnpm build,pnpm lint,pnpm format— all pass.packages/models/src/compliance.spec.ts(6 tests).apps/gateway/src/api.spec.ts: a non-compliant provider is blocked (403 + security event) and a compliant provider is allowed (200).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
allowed_domains/blocked_domainsand improved native messages server-side tool handling.