Repository navigation
feat(compliance): add no-stealth-providers option - #3340
Conversation
Adds a `blockStealthProviders` toggle to the enterprise provider compliance policy so stealth providers can be excluded explicitly, without having to enable an unrelated certification requirement. Stealth providers have a null dataPolicy/headquarters, so they already fail every existing requirement fail-closed — this makes the exclusion intentional rather than incidental. Moves `isStealthProvider` from helpers.ts into providers.ts so the compliance predicates can use it without a circular import, and adds `getProviderRequirementFailures` for the dashboard pickers, which need requirement-level failures without the fine-grained provider lists. Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughThe PR adds optional stealth-provider blocking to provider compliance policies. Model logic detects providers without required ChangesStealth provider compliance
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OrganizationRoute
participant ComplianceClient
participant ProviderRequirements
participant StealthDetection
OrganizationRoute->>ComplianceClient: Provide blockStealthProviders policy
ComplianceClient->>ProviderRequirements: Evaluate provider requirements
ProviderRequirements->>StealthDetection: Inspect baseUrl configuration
StealthDetection-->>ProviderRequirements: Return stealth status
ProviderRequirements-->>ComplianceClient: Return compliance failures
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Actionable comments posted: 1
🤖 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/docs/content/features/compliance.mdx`:
- Around line 31-36: Revise the fail-closed statement in the compliance
requirements documentation to apply only to certification, data-policy, and
headquarters requirements. Preserve the behavior that a non-stealth provider
with unknown attributes can pass when only the No stealth providers requirement
is active.
🪄 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 Plus
Run ID: 84b65dbd-9722-4072-9745-f1f8d2215838
📒 Files selected for processing (7)
apps/api/src/routes/organization.tsapps/docs/content/features/compliance.mdxapps/ui/src/app/dashboard/[orgId]/org/compliance/compliance-client.tsxpackages/models/src/compliance.spec.tspackages/models/src/helpers.tspackages/models/src/providers.spec.tspackages/models/src/providers.ts
| | **No stealth providers** | it is not a stealth provider | | ||
|
|
||
| Every requirement is **fail-closed**: a provider passes only if its published data policy explicitly satisfies the requirement. If an attribute is unknown for a provider, that provider is treated as non-compliant. | ||
|
|
||
| Stealth providers are undisclosed platforms that LLM Gateway routes to without naming the operator, so their data policy and headquarters are unknown. That already makes them fail every certification and data-policy requirement above, but **No stealth providers** excludes them explicitly — useful when you want them gone without turning on any other requirement. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Limit the fail-closed statement to attribute requirements.
Line 33 says every requirement needs an explicit published data policy. A non-stealth provider with unknown data-policy attributes passes when blockStealthProviders is the only active requirement. Update the statement to cover certification, data-policy, and headquarters requirements only.
🤖 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/docs/content/features/compliance.mdx` around lines 31 - 36, Revise the
fail-closed statement in the compliance requirements documentation to apply only
to certification, data-policy, and headquarters requirements. Preserve the
behavior that a non-stealth provider with unknown attributes can pass when only
the No stealth providers requirement is active.
## Summary
The video pipeline forwards upstream provider error bodies to the client
**verbatim**, skipping the stealth redaction every other pipeline
already applies. `fetchUpstreamJson`
(`apps/gateway/src/videos/videos.ts:2788`) throws `HTTPException(status,
{ message: body.error.message })` on `!response.ok` — with the full raw
text when the error isn't JSON — and re-forwards `body.msg` untouched on
the application-error branch; `app.ts`'s `onError` then renders that
message to the client via `renderGatewayError`. Unlike `chat`, `rerank`
and `embeddings`, `videos.ts` never calls `shouldRedactProviderError`.
For stealth providers this leaks exactly what theopenco#3340 and the
`stealth-provider-errors.ts` invariant exist to protect: provider
identity, hostnames and vendor markings inside the raw error body (e.g.
`quota exceeded at https://<secret-host>` from `avalanche`, whose base
URL is a deployment secret).
## Fix
Mirror the sibling pattern (`rerank.ts:1060`, `embeddings.ts:1347`):
- New pure helper `videos/upstream-error.ts`:
`clientFacingUpstreamMessage(providerId, statusCode, rawMessage)` —
`redactedProviderErrorText(statusCode)` when
`shouldRedactProviderError(providerId)`, the raw message unchanged
otherwise. Same comment as the sibling sites.
- `fetchUpstreamJson` takes an optional `providerId` and routes both
throw branches through the helper (the `Upstream provider error
(<status>)` fallback was already generic and stays as-is). Internal
`logger.warn` keeps the full body, consistent with the rest of the
codebase.
- All 11 call sites pass `providerContext.providerId` (each already has
it in scope). Non-stealth providers are byte-identical to before.
## Tests
New pure spec `videos/upstream-error.spec.ts` (DB-light, no harness):
- stealth (`avalanche`): the raw message never reaches the client — the
response is exactly `redactedProviderErrorText(500)` and contains
neither the secret host nor the original message;
- non-stealth (`openai`): raw message passes through unchanged;
- `undefined` provider: raw message passes through unchanged.
Adversarial cycle run: commenting the guard makes the stealth test
**fail** (`expected 'quota exceeded at https://plataforma-…' to be
'Upstream provider error (500…)'`); restoring it passes again.
## Verification
- New spec 3/3 ✅ · `normalize-streaming-error.spec.ts` 13/13 ✅ (the
stealth sibling suites stay green)
- `turbo run build --filter=gateway` ✅ (10/10 tasks — full typecheck of
`videos.ts` + helper + spec)
- eslint ✅ · prettier ✅ (touched files)
Not verified: end-to-end against a real stealth provider (no DB/Docker
on this machine) — the `fetchUpstreamJson` → `HTTPException` →
`renderGatewayError` path is verified by reading, and the redaction
decision itself by the new unit tests.
---
Disclosure: an AI coding assistant helped survey the pipeline and draft
the test scaffolding. The diff is small and was verified the slow way: I
read every call site, confirmed `videos.ts` is the only pipeline without
the guard, and ran the revert-the-guard check above to prove the test
actually catches the leak. I own the change and will follow up on review
comments.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Sensitive upstream provider details are now hidden from error messages
for supported providers.
* Video creation, media uploads, and background processing return
consistent generic errors when provider requests fail.
* Non-sensitive provider errors continue to display their original
details.
* Failed video status responses now include the appropriate sanitized
error message.
* **Tests**
* Added coverage for HTTP, application-level, and background
video-processing error redaction.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Follow-up: async video-job redaction
This update also redacts stealth-provider failures before the worker
persists the public job error, so the asynchronous status endpoint
cannot expose upstream response text or network details.
It adds a full `POST /v1/videos` leak-regression case and a
worker-to-status regression case, moves the HTTP helper into the common
stealth-error module, and requires provider context at every video
upstream request.
---------
Co-authored-by: Luca Steeb <contact@luca-steeb.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a No stealth providers toggle to the enterprise provider compliance policy (Settings → Compliance), so stealth providers can be excluded explicitly instead of only incidentally.
Stealth providers (Glacier, Iceberg, Granite, …) have a
nulldataPolicyandheadquarters, so they already fail every existing requirement fail-closed. But that only holds if at least one other requirement is on — an org that wants a policy consisting solely of "no undisclosed platforms" previously had no way to express it. This makes the exclusion intentional and self-documenting in the impact preview.Changes
packages/models:blockStealthProvidersonProviderCompliancePolicy, plus the matchingComplianceFailureReason. Because the check is provider-level (not data-policy-level), it lives in a newgetProviderRequirementFailures(provider, policy)thatgetProviderComplianceFailurescomposes — the dashboard pickers need requirement-level failures without the fine-grained provider lists, and now pick up the stealth reason too.isStealthProvidermoves fromhelpers.tstoproviders.ts(helpers imports providers, so the compliance predicates couldn't call it without a circular import). Same export surface via the package index; only the internal import paths in specs changed.apps/api: accept the new field inproviderCompliancePolicySchema. The policy is a JSON column, so no migration.apps/ui: new requirement switch +"Stealth provider"blocked-chip reason.apps/docs: requirement table row and a paragraph on why the option exists.Gateway enforcement needed no change — it routes through
isProviderCompliant, which takes the fullProviderDefinition. Custom-provider attestations are unaffected: a self-attested deployment is never stealth.Testing
blockStealthProviderssuite inpackages/models/src/compliance.spec.tscovering the enabled/disabled/toggle-off matrix, non-stealth providers, every catalogue stealth provider, and attestation non-applicability.pnpm test:unit— 3475 passed, 2 skipped.pnpm build— clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests