adds support for CA in proxy settings #1137
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds PEM CA certificate support for proxy TLS: new proxy config fields and transport schema, frontend form and types, and backend TLS utilities that append provided CA to system roots and apply the TLS config to proxy clients; also embeds proxy credentials into HTTP/SOCKS5 proxy URLs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Potential focus areas:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📚 Learning: 2025-12-09T17:07:42.007ZApplied to files:
📚 Learning: 2025-12-19T09:26:54.961ZApplied to files:
🧬 Code graph analysis (1)core/schemas/provider.go (2)
🔇 Additional comments (9)
Comment |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/docs.json (1)
8-42: Documentation changes appear unrelated to CA certificate support.These changes (enterprise banner, navigation restructuring) don't seem related to the PR's stated objective of adding CA support in proxy settings. Consider splitting unrelated documentation/marketing changes into a separate PR for cleaner history and easier review.
ui/lib/types/schemas.ts (1)
258-258: Consider adding PEM format validation.The
ca_cert_pemfield accepts any string without format validation. Adding a basic check for valid PEM certificate structure would catch user errors early and improve the user experience.🔎 Proposed validation refinement
You can add a
.refine()to validate basic PEM structure. For example, forproxyConfigSchema:export const proxyConfigSchema = z .object({ type: proxyTypeSchema, url: z.url("Must be a valid URL"), username: z.string().optional(), password: z.string().optional(), - ca_cert_pem: z.string().optional(), + ca_cert_pem: z.string().optional() + .refine((val) => { + if (!val || val.trim() === "") return true; + return val.includes("-----BEGIN CERTIFICATE-----") && val.includes("-----END CERTIFICATE-----"); + }, { message: "Must be a valid PEM-encoded certificate" }), })Apply similar refinements to
proxyFormConfigSchema(line 286) andglobalProxyConfigSchema(line 634).Also applies to: 286-286, 634-634
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
core/providers/utils/utils.go(3 hunks)core/schemas/provider.go(1 hunks)docs/docs.json(3 hunks)transports/config.schema.json(1 hunks)ui/app/workspace/config/views/proxyView.tsx(1 hunks)ui/app/workspace/providers/fragments/proxyFormFragment.tsx(5 hunks)ui/lib/types/config.ts(2 hunks)ui/lib/types/schemas.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
ui/lib/types/config.tsui/app/workspace/providers/fragments/proxyFormFragment.tsxtransports/config.schema.jsoncore/schemas/provider.gocore/providers/utils/utils.goui/lib/types/schemas.tsui/app/workspace/config/views/proxyView.tsxdocs/docs.json
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/schemas/provider.gocore/providers/utils/utils.go
📚 Learning: 2025-12-19T09:26:54.961Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/utils/utils.go:1050-1051
Timestamp: 2025-12-19T09:26:54.961Z
Learning: Update streaming end-marker handling so HuggingFace is treated as a non-[DONE] provider for backends that do not emit a DONE marker (e.g., meta llama on novita). In core/providers/utils/utils.go, adjust ProviderSendsDoneMarker() (or related logic) to detect providers that may not emit DONE and avoid relying on DONE as the sole end signal. Add tests to cover both DONE-emitting and non-DONE backends, with clear documentation in code comments explaining the rationale and any fallback behavior.
Applied to files:
core/providers/utils/utils.go
🧬 Code graph analysis (2)
ui/app/workspace/providers/fragments/proxyFormFragment.tsx (2)
ui/components/ui/form.tsx (5)
FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormDescription(164-164)FormMessage(165-165)ui/components/ui/textarea.tsx (1)
Textarea(18-18)
core/schemas/provider.go (2)
core/providers/gemini/types.go (1)
Type(782-782)ui/lib/types/config.ts (1)
ProxyType(126-126)
🪛 ast-grep (0.40.0)
core/providers/utils/utils.go
[warning] 186-188: MinVersionis missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{
RootCAs: rootCAs,
}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(missing-ssl-minversion-go)
🔇 Additional comments (9)
core/schemas/provider.go (1)
157-163: LGTM!The
CACertPEMfield addition is consistent with other proxy config fields and aligns with the JSON schema changes intransports/config.schema.json. The field correctly usesjson:"ca_cert_pem"to match the snake_case convention used throughout.transports/config.schema.json (1)
1845-1849: LGTM!The
ca_cert_pemproperty is correctly added as an optional string field. The description clearly explains its purpose for SSL-intercepting proxies, and it aligns with the Go struct definition incore/schemas/provider.go.core/providers/utils/utils.go (1)
159-167: CA certificate integration looks correct.The logic properly:
- Only configures TLS when
CACertPEMis provided- Logs a warning if CA setup fails rather than failing silently
- Continues to return the configured client regardless of CA setup success
One consideration: if CA setup fails, the client proceeds without the custom CA, which may cause unexpected TLS errors when connecting through the proxy. Depending on use case, you may want to return early or make this behavior configurable.
ui/lib/types/config.ts (2)
128-135: LGTM!The
ca_cert_pemoptional field correctly mirrors the GoProxyConfig.CACertPEMfield and maintains type consistency across the frontend-backend boundary.
269-283: LGTM!The
ca_cert_pemfield is correctly added as optional toGlobalProxyConfig. Note thatDefaultGlobalProxyConfigdoesn't include this field, which is appropriate since it's optional and should default to undefined.ui/app/workspace/config/views/proxyView.tsx (1)
259-282: LGTM! Clean implementation of the CA certificate field.The CA certificate input is properly integrated into the Advanced Settings section with appropriate styling (monospace font for PEM format), clear placeholder text, and consistent behavior with other form fields (disabled when proxy is not enabled).
ui/app/workspace/providers/fragments/proxyFormFragment.tsx (3)
4-4: LGTM! Proper imports for the new form field.The necessary UI components (
FormDescriptionandTextarea) are correctly imported to support the CA certificate input field.Also applies to: 7-7
37-37: LGTM! Consistent form state management for ca_cert_pem.The
ca_cert_pemfield is properly wired through the form lifecycle:
- Initialized in
defaultValueswith fallback to empty string- Included in
resetto handle provider changes- Correctly submitted with empty string converted to
undefinedAlso applies to: 53-53, 68-68
159-180: LGTM! CA certificate field implementation matches the global proxy view.The form field implementation is consistent with the global proxy configuration view, using the same styling (monospace font, 6 rows), placeholder format, and description text.
Pratham-Mishra04
force-pushed
the
12-19-adds_support_for_ca_in_proxy_settings
branch
from
46b92ea to
b149ea2
Compare
Merge activity
|
Pratham-Mishra04
deleted the
12-19-adds_support_for_ca_in_proxy_settings
branch
Summary
Enhanced proxy support by adding custom CA certificate capabilities and HTTP proxy authentication, allowing Bifrost to work with SSL-intercepting proxies and authenticated proxies.
Changes
ca_cert_pemfieldType of change
Affected areas
How to test
Breaking changes
Security considerations
Checklist