Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAcross 11 GitHub workflow files, the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~10 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
Contributor
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
Contributor
Author
Merge activity
|
Contributor
Confidence Score: 5/5Safe to merge — changes are mechanical removals and a deliberate schema relaxation with no breaking changes. All findings are P2 style/hardening suggestions. The schema relaxation is intentional and correctly described. No logic errors, data-integrity issues, or security regressions are introduced. helm-charts/bifrost/values.schema.json — minor co-validation gap between Important Files Changed
Reviews (1): Last reviewed commit: "fixed helm schema fix" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
716
to
719
| "required": [ | ||
| "dimension", | ||
| "keys", | ||
| "provider" | ||
| "dimension" | ||
| ] | ||
| } |
Contributor
There was a problem hiding this comment.
provider and keys
Now that keys and provider are no longer required, a user could supply a provider without keys (or vice versa) and the schema would accept it silently. The keys field description already notes it is "required for semantic caching", so this is an expected pairing.
Consider adding an if/then conditional (similar to the pattern used elsewhere in this file) to enforce that when provider is present, keys must also be present:
{
"if": {
"properties": { "config": { "required": ["provider"] } },
"required": ["config"]
},
"then": {
"properties": {
"config": { "required": ["keys"] }
}
}
}This is a minor hardening suggestion — the relaxation itself (supporting dimension-only for direct caching) is correct.
akshaydeo
added a commit
that referenced
this pull request
Mar 30, 2026
* v1.4.17-release-cut (#2368) * fixed helm schema fix (#2369) ## Summary Remove Bullfrog security monitoring from GitHub Actions workflows and update Helm chart schema validation requirements. ## Changes - Removed `bullfrogsec/bullfrog@7bc9b6e13e2dd9cbe5861f33bc26dc6bdb9d9ed2` action with `egress-policy: audit` from all GitHub Actions workflows - Updated Helm chart values schema to only require `dimension` field instead of `dimension`, `keys`, and `provider` for config objects ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify that GitHub Actions workflows execute successfully without the Bullfrog security step: ```sh # Trigger any workflow to ensure it runs without errors # Check that Helm chart validation accepts configs with only dimension field helm lint helm-charts/bifrost/ ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations This change removes network egress monitoring from CI/CD pipelines. Ensure alternative security measures are in place if network monitoring is still required. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix migration tests (#2371) * framework: bump core to v1.4.15 --skip-pipeline * plugins/governance: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/jsonparser: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/litellmcompat: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/logging: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/maxim: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/mocker: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/otel: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/semanticcache: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/telemetry: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * test fixes (#2374) ## Summary Adds support for testing streaming multiple tool calls functionality across LLM providers. This addresses the need to differentiate between providers that support multiple tool calls in streaming mode versus those that only return one tool call at a time during streaming. ## Changes - Added `MultipleToolCallsStreaming` field to `TestScenarios` struct to track streaming multiple tool calls capability - Updated all provider test configurations to enable the new streaming multiple tool calls feature - Added conditional test skipping in multiple tool calls streaming tests when providers don't support this functionality - Refined streaming response validation expectations to handle consolidated responses from chunks more accurately - Updated provider-specific validation expectations for Cohere and Parasail to reflect their actual response formats ## Type of change - [x] Feature - [x] Refactor ## Affected areas - [x] Core (Go) - [x] Providers/Integrations ## How to test Run the LLM provider tests to validate streaming multiple tool calls functionality: ```sh # Core/Transports go version go test ./... # Test specific provider streaming multiple tool calls go test ./core/providers/openai -v -run TestOpenAI go test ./core/providers/anthropic -v -run TestAnthropic ``` The tests will automatically skip streaming multiple tool calls scenarios for providers that don't support this feature. ## Screenshots/Recordings N/A - Backend testing enhancement only. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications - this is a testing infrastructure enhancement. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * transports: update dependencies --skip-pipeline * Adds changelog for v1.4.18 --skip-pipeline * [StepSecurity] Apply security best practices (#2372) * [StepSecurity] Apply security best practices Signed-off-by: StepSecurity Bot <bot@stepsecurity.io> * Update .github/workflows/codeql.yml Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * cr fixes * fixed cr comments 2 --------- Signed-off-by: StepSecurity Bot <bot@stepsecurity.io> Co-authored-by: Akshay Deo <akshay@akshaydeo.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * removed codeql to fallback on default setup (#2376) ## Summary Removes the CodeQL security analysis workflow from the GitHub Actions configuration. This eliminates automated static code analysis for Go, JavaScript, and Python languages that was previously running on pushes, pull requests, and weekly schedules. ## Changes - Deleted `.github/workflows/codeql.yml` which contained the complete CodeQL workflow configuration - Removed automated security scanning for Go, JavaScript, and Python codebases - Eliminated the scheduled weekly security analysis runs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify that CodeQL workflow is no longer present in the repository: ```sh # Confirm the workflow file has been removed ls -la .github/workflows/ # Should not show codeql.yml # Check GitHub Actions tab to ensure CodeQL runs are no longer scheduled # Navigate to repository Actions tab and verify no CodeQL workflows appear ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations This change removes automated security analysis from the CI/CD pipeline. The repository will no longer benefit from CodeQL's static analysis capabilities for detecting security vulnerabilities, code quality issues, and potential bugs in Go, JavaScript, and Python code. Consider alternative security scanning solutions if this workflow was providing valuable security insights. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * Update snyk.yml (#2380) * fips base image (#2310) ## Summary Updates the transports Docker configuration to use a FIPS-compliant base image and removes the built-in health check mechanism. ## Changes - Replaced `alpine:3.23.3` base image with `dhi.io/alpine-base:3.23-alpine3.23-fips` for FIPS compliance - Removed the Docker HEALTHCHECK directive that was monitoring the `/health` endpoint ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify the Docker image builds successfully with the new base image and that the application starts correctly without the health check. ```sh # Build the Docker image docker build -t transports-test ./transports # Run the container docker run -p 8080:8080 transports-test # Verify the application is running curl http://localhost:8080/health ``` ## Screenshots/Recordings N/A ## Breaking changes - [x] Yes - [ ] No The removal of the Docker HEALTHCHECK may affect container orchestration systems that rely on Docker's built-in health checking. External health monitoring will need to be configured if required. ## Related issues N/A ## Security considerations This change enhances security by adopting a FIPS-compliant base image, which provides cryptographic modules that meet Federal Information Processing Standards. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable * fixed snyk failure and enterprise update (#2382) ## Summary Enhances the model catalog to extract and cache max output tokens from pricing data, improves Snyk workflow reliability by checking for SARIF file existence before upload, and updates security documentation to reflect full SHA pinning for the enterprise repository. ## Changes - Added `MaxOutputTokens` field to `PricingEntry` struct to capture model parameter limits from pricing datasheet - Implemented `populateModelParamsFromPricing()` function to extract max output tokens and populate the model params cache - Refactored model name extraction logic into reusable `extractModelName()` utility function - Enhanced Snyk workflow conditions to only upload SARIF files when they exist, preventing upload failures - Updated security documentation to reflect that bifrost-enterprise now uses full SHA pinning (100% coverage) ## Type of change - [x] Feature - [x] Chore/CI - [x] Documentation ## Affected areas - [x] Core (Go) - [x] Docs ## How to test Verify model catalog functionality and pricing data processing: ```sh # Core functionality go version go test ./framework/modelcatalog/... # Test pricing sync with max_output_tokens go test -v ./framework/modelcatalog/ -run TestSyncPricing # Verify Snyk workflow changes in CI # Check that SARIF upload steps only run when files exist ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations - Improves CI security by preventing potential failures in SARIF upload steps - Documents enhanced security posture with full SHA pinning in enterprise repository ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix snyk failures (#2385) ## Summary Improves Docker container security and reliability by migrating from custom base image to standard Alpine, adding health checks, and enhancing Snyk security scanning. ## Changes - **Docker base image migration**: Replaced `bifrosthq/dhi-alpine-base:3.22-fips_bifrost-v27032026` with standard `alpine:3.23.3` for better security and maintainability - **Added health checks**: Implemented HTTP health check endpoint monitoring with 30s intervals and proper retry logic - **Enhanced user management**: Consolidated user creation and permission setup into single RUN command for better layer optimization - **Improved Snyk scanning**: Added build step before security scanning and excluded `examples` and `tests/scripts` directories from vulnerability analysis - **Runtime dependencies**: Explicitly installed required CGO runtime libraries (musl, libgcc, ca-certificates, wget) ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Validate Docker builds and container health: ```sh # Build and test standard Dockerfile docker build -f transports/Dockerfile -t bifrost:test . docker run -d --name bifrost-test -p 8080:8080 bifrost:test docker ps # Should show healthy status after ~35s curl http://localhost:8080/health # Should return 200 OK # Build and test local development Dockerfile docker build -f transports/Dockerfile.local -t bifrost:local . docker run -d --name bifrost-local -p 8081:8080 bifrost:local docker ps # Should show healthy status after ~35s # Test Snyk workflow make build # Verify build step works # Run Snyk scan (requires SNYK_TOKEN) ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations - Migrated from custom base image to well-maintained Alpine Linux for better security patching - Enhanced Snyk scanning excludes test directories to focus on production code vulnerabilities - Health check endpoint provides better container monitoring capabilities - Explicit runtime dependency management reduces attack surface ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix: add explicit type="button" to navigation and action buttons (#2353) ## Summary Added explicit `type="button"` attributes to Button components in log detail sheets and plugin sequence forms to prevent unintended form submissions when these buttons are clicked. ## Changes - Added `type="button"` to navigation buttons (Previous/Next) in log detail sheets - Added `type="button"` to dropdown menu trigger buttons in log detail sheets - Added `type="button"` to the Save Sequence button in plugin sequence sheet - Added `type="button"` to the Copy button in CEL rule builder This prevents these buttons from accidentally triggering form submissions when used within forms, ensuring they only perform their intended click actions. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test Verify that buttons in log detail sheets and plugin forms work correctly without triggering form submissions: 1. Open log detail sheets and test navigation buttons 2. Test dropdown menu triggers in log sheets 3. Test plugin sequence save functionality 4. Test CEL rule builder copy button ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings N/A - This is a behavioral fix without visual changes. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None - this change only affects button behavior to prevent unintended form submissions. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix: prevent edit mode activation when clicking interactive elements in message views (#2377) ## Summary Prevents edit mode from being triggered when clicking on interactive elements (buttons, links, or elements with button role) within message views. This fixes the issue where clicking on buttons or links inside messages would unintentionally activate edit mode. ## Changes - Added event target checking in onClick handlers for assistant, system, and user message views - Modified click handlers to check if the clicked element is within a button, link, or element with button role using `closest()` method - Restructured conditional logic for better readability and early returns ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test 1. Navigate to a message view with interactive elements (buttons, links) 2. Click on buttons or links within the message content 3. Verify that edit mode is not triggered when clicking interactive elements 4. Click on non-interactive areas of the message to confirm edit mode still works 5. Test across all message types (assistant, system, user) ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications - this is a UI interaction improvement. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * refactor: extract copy-to-clipboard logic into reusable hook (#2379) ## Summary Refactored clipboard copy functionality across the UI by creating a reusable `useCopyToClipboard` hook to replace duplicate clipboard handling code. ## Changes - Created a new `useCopyToClipboard` hook that provides consistent clipboard functionality with customizable success/error messages and automatic reset of copied state - Replaced inline clipboard handling code across 11 components with the new hook - Removed direct `navigator.clipboard.writeText()` calls and manual toast notifications - Added support for customizable messages and reset delays in the hook ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test Test clipboard functionality across the affected components: ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` 1. Navigate to API Keys page and test copying curl examples 2. Open log details and test copying request IDs and request bodies 3. Test copy functionality in routing rules CEL builder 4. Test copy functionality in virtual keys table 5. Test copy functionality in Prometheus configuration 6. Verify all copy actions show appropriate toast messages ## Screenshots/Recordings No visual changes - functionality remains the same with improved code organization. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications - maintains existing clipboard functionality without changes to security model. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * refactor: replace lazy query with polling query and fix overflow in log details sheet (#2381) ## Summary Improves log detail sheet reliability by switching from lazy loading to automatic polling for failed log fetches and fixes UI overflow issues in the sheet header. ## Changes - Replaced `useLazyGetLogByIdQuery` with `useGetLogByIdQuery` for automatic data fetching when the sheet opens - Added polling mechanism that retries every 2 seconds when log fetch fails, stopping when successful - Fixed horizontal overflow issues in the sheet header by adding `overflow-x-hidden` classes - Exported `useGetLogByIdQuery` hook from the logs API ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test 1. Open the logs page and click on a log entry to view details 2. Verify the log details load automatically without manual triggering 3. Test with network issues or slow responses to confirm polling retry behavior 4. Check that long request IDs don't cause horizontal overflow in the sheet header ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None - this change only affects UI data fetching patterns and styling. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * merge-branch-main-into-v-1-5-0 resolve * merge-branch-main-into-v-1-5-0 resolve --------- Signed-off-by: StepSecurity Bot <bot@stepsecurity.io> Co-authored-by: Akshay Deo <akshay@akshaydeo.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: StepSecurity Bot <bot@stepsecurity.io> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove Bullfrog security monitoring from GitHub Actions workflows and update Helm chart schema validation requirements.
Changes
bullfrogsec/bullfrog@7bc9b6e13e2dd9cbe5861f33bc26dc6bdb9d9ed2action withegress-policy: auditfrom all GitHub Actions workflowsdimensionfield instead ofdimension,keys, andproviderfor config objectsType of change
Affected areas
How to test
Verify that GitHub Actions workflows execute successfully without the Bullfrog security step:
Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
This change removes network egress monitoring from CI/CD pipelines. Ensure alternative security measures are in place if network monitoring is still required.
Checklist
docs/contributing/README.mdand followed the guidelines