Skip to content

v1..5.7 - #3910

Merged
akshaydeo merged 7 commits into
mainfrom
dev
May 30, 2026
Merged

v1..5.7#3910
akshaydeo merged 7 commits into
mainfrom
dev

Conversation

@akshaydeo

Copy link
Copy Markdown
Contributor

🔒 Security

  • Go Dependency CVE Remediation — Updated golang.org/x dependencies flagged by Docker Scout, clearing 20 advisories (severity up to 10.0): crypto v0.49.0 → v0.52.0, net v0.52.0 → v0.55.0, sys v0.42.0 → v0.45.0, text v0.35.0 → v0.37.0, term v0.41.0 → v0.43.0 (cli). Verified with govulncheck against the live Go vulnerability database: zero vulnerabilities remain in any module (docker scout fixes #3900)
  • Hardened Container Image — Removed the standalone GNU wget package from the Alpine runtime image, eliminating CVE-2025-69194 (8.8); the HEALTHCHECK now uses the built-in busybox wget applet, with no functional change

🐞 Fixed

akshaydeo and others added 6 commits May 30, 2026 20:23
…3307)

* [fix]: SGL provider - send Authorization header on streaming requests

The streaming entry points (ChatCompletionStream, TextCompletionStream)
passed nil for the authHeader parameter to the shared OpenAI streaming
helpers, so no Authorization header was attached to outbound streaming
requests. SGLang servers configured with --api-key always require the
header and returned 401 on streaming while non-streaming requests
worked. The non-streaming OpenAI helper takes the Key directly and
builds the header itself; the streaming helper requires the caller to
build it. Mirror the vLLM pattern (core/providers/vllm/vllm.go) and
construct the auth header from key.Value when set.

Affected packages:
- core/providers/sgl/sgl.go - build authHeader for both streaming paths
- core/providers/sgl/chat_test.go - regression tests asserting the
  Authorization header reaches the upstream on chat and text streams
- core/changelog.md - changelog entry

* [fix]: SGL streaming tests - cancel drain goroutine on test completion

The drain goroutines spawned to consume streamChan in
TestChatCompletionStream_SetsAuthorizationHeader and
TestTextCompletionStream_SetsAuthorizationHeader had no cancellation
path: if the streaming pipeline failed to close the channel (e.g. on a
test timeout), the goroutines would leak into the test process.

Replace the inline `go func() { for range streamChan {} }()` with a
shared drainStream helper that selects on both the channel and a `done`
channel closed via t.Cleanup, so the goroutine always exits when the
test completes regardless of channel state.

Addresses Greptile review feedback on PR #3307.

---------

Co-authored-by: Akshay Deo <akshay@akshaydeo.com>
## Summary

Adds Bearer token authentication support to the Ollama provider's streaming endpoints. Previously, the streaming methods for text completion and chat completion always passed `nil` for the auth header, meaning API keys configured for Ollama were silently ignored during streaming requests.

## Issues

Closes #3905

## Changes

- When a non-empty key value is present, a `Bearer` token `Authorization` header is now constructed and passed to the OpenAI-compatible streaming handlers for both `TextCompletionStream` and `ChatCompletionStream`
- If no key is configured, the auth header remains `nil`, preserving backward compatibility with unauthenticated local Ollama instances

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

Configure an Ollama provider with an API key (e.g., when using a hosted or authenticated Ollama instance) and issue a streaming chat or text completion request. Verify the `Authorization: Bearer <key>` header is included in the outgoing request.

```sh
go test ./core/providers/ollama/...
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

API keys are now correctly forwarded as Bearer tokens in streaming requests to Ollama. Ensure keys are stored and retrieved securely via the existing key management mechanism, as they will now be included in outbound HTTP headers for streaming calls.

## 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

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Ollama streaming requests now support authentication via bearer tokens for both text and chat completions, enabling proper token-based authentication when API keys are provided.

<!-- review_stack_entry_start -->

[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/3906?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This release (core v1.5.15, framework v1.3.15) fixes missing `Authorization` header forwarding on streaming requests for the Ollama and SGL providers, ensuring authenticated requests behave correctly during streaming.

- Ollama streaming text and chat completion requests now correctly forward the configured API key as an `Authorization: Bearer` header (#3906)
- SGL provider now sends the `Authorization` header on streaming requests (#3307) (thanks [@hensapir](https://github.com/hensapir)!)
- Governance and Logging APIs: removed the `from_memory` query parameter; virtual key and config list APIs now return consistent DB-backed results, with VK names batch-fetched in a single query (#3903)
- Bumped core to v1.5.15, framework to v1.3.15, transports to v1.5.7, and all dependent plugins to their respective patch versions

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

Validate that Ollama and SGL streaming requests include the `Authorization: Bearer` header when an API key is configured.

```sh
go version
go test ./...
```

Configure an Ollama or SGL provider with an API key and issue a streaming chat or text completion request. Inspect outbound request headers to confirm `Authorization: Bearer <key>` is present.

- [ ] Yes
- [x] No

Closes #3906
Closes #3307
Closes #3903

These fixes ensure that API keys configured for Ollama and SGL providers are correctly forwarded on streaming requests. Previously, the `Authorization` header was silently dropped on streaming paths, meaning requests could reach upstream providers without authentication credentials.

- [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

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

* **Bug Fixes**
  * Fixed authorization header handling for Ollama streaming requests.
  * Fixed authorization header forwarding for SGL provider streaming requests.
  * Improved consistency in virtual key and configuration list API responses by removing unnecessary query parameters.

* **Chores**
  * Updated component versions across the platform.

<!-- review_stack_entry_start -->

[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/3907?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@akshaydeo
akshaydeo requested a review from a team as a code owner May 30, 2026 15:10
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ hensapir
❌ akshaydeo
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6a877793-5e12-4ca8-a6ff-f4e10782b0b5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

Comment @coderabbitai help to get the list of available commands and usage tips.

## Summary

Remediates Docker Scout CVE findings by upgrading transitive `golang.org/x` dependencies and removing the standalone GNU `wget` package from Alpine runtime images, replacing it with the built-in busybox `wget` applet.

## Changes

- Bumped `golang.org/x` transitive dependencies (`crypto`, `net`, `sys`, `text`, `term`) across all modules to clear 20 Docker Scout advisories (severity up to 10.0), verified clean with `govulncheck`
- Removed standalone `wget` package from Alpine runtime images in `Dockerfile` and `Dockerfile.local`, eliminating CVE-2025-69194 (CVSS 8.8)
- Updated `HEALTHCHECK` command from `wget --no-verbose --tries=1` to `wget -q` to use busybox-compatible flags with no functional change in behavior

## Type of change

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [x] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
# Verify no vulnerabilities remain
govulncheck ./...

# Build Docker image and confirm wget healthcheck works
docker build -f transports/Dockerfile -t gateway-test .
docker run --rm gateway-test
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

Closes #3900

## Security considerations

- Clears 20 Docker Scout CVE advisories on `golang.org/x` packages, with severities up to 10.0
- Removes CVE-2025-69194 (CVSS 8.8) by eliminating the standalone GNU `wget` package; busybox `wget` is used instead and is not affected by this CVE

## 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
@akshaydeo
akshaydeo merged commit 5fafce1 into main May 30, 2026
16 of 17 checks passed
@greptile-apps

greptile-apps Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

Safe to merge; all functional changes are targeted bug fixes with corresponding tests, and the dependency and container updates are verified clean.

The streaming auth fix is correct and consistent across both providers, but Ollama's path has no regression test while SGL's does. Everything else — the governance from_memory removal, the DAC-scoped cache fix, the MCP batch-fetch, and the weaviate readiness rework — is well-covered by new unit tests or explicit script-level checks.

core/providers/ollama/ollama.go — the streaming auth change is untested; consider adding a test analogous to the SGL regression tests.

Important Files Changed

Filename Overview
core/providers/ollama/ollama.go Streaming text and chat completion now forward the API key as Authorization: Bearer; fix is correct but lacks regression tests unlike the parallel SGL fix.
core/providers/sgl/sgl.go Streaming paths now correctly build the Authorization header from key.Value; backed by two new regression tests in chat_test.go.
core/providers/sgl/chat_test.go Adds testLogger stub, streaming client setup, and two regression tests that verify the Authorization header is sent on ChatCompletionStream and TextCompletionStream.
transports/bifrost-http/handlers/governance.go Removes from_memory parameter from all governance list/get handlers; all paths now go to the DB-backed ConfigStore, with VK names batch-fetched in MCP handler.
transports/bifrost-http/handlers/logging.go Introduces shouldUseFilterDataCache that correctly excludes DAC-scoped requests from the shared filterdata cache, preventing cross-tenant data exposure.
transports/bifrost-http/handlers/mcp.go MCP VK name lookup migrated from in-memory governance data to a single batch DB query via GetRedactedVirtualKeys; errors are logged and handled gracefully.
.github/workflows/configs/docker-compose.yml Removes the weaviate Docker healthcheck; all three test scripts now poll the readiness endpoint directly via curl with a 60-second timeout, which replaces the previous in-compose check.

Comments Outside Diff (1)

  1. core/providers/ollama/ollama.go, line 156-177 (link)

    P2 Missing Ollama streaming auth regression tests

    The SGL auth fix ships with two new regression tests (TestChatCompletionStream_SetsAuthorizationHeader / TestTextCompletionStream_SetsAuthorizationHeader in core/providers/sgl/chat_test.go) that verify the Authorization: Bearer header is forwarded on streaming requests. The equivalent Ollama fix in both TextCompletionStream and ChatCompletionStream is structurally identical but has no corresponding test coverage. If the auth-header wiring ever regresses in the Ollama path, there is nothing to catch it at the unit level.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "changelogs (#3909)" | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants