Skip to content

unauthenticated proxy blocks - #5778

Open
akshaydeo wants to merge 9 commits into
devfrom
block-unauthenticated-proxy-config-writes
Open

unauthenticated proxy blocks#5778
akshaydeo wants to merge 9 commits into
devfrom
block-unauthenticated-proxy-config-writes

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Setting the outbound proxy URL is at least as sensitive as setting a provider's base URL: once enabled, the proxy replaces the HTTP client's Dial function entirely, routing every outbound provider request — including Authorization and x-api-key headers — through the attacker-chosen host. Without this fix, an unauthenticated caller on the network could set the proxy URL while dashboard auth is disabled or unconfigured, since the auth middleware's fail-open branch previously granted the same IsLocalAdminContextKey flag as a genuinely authenticated session.

Changes

  • Introduced BifrostContextKeyAuthBypassed context key, set exclusively in the auth middleware's fail-open branch (when authConfig == nil or auth is disabled), distinct from IsLocalAdminContextKey which is also true for real authenticated sessions.
  • Added isAuthBypassed helper in the config handler to read this key.
  • Gated the "enable proxy with URL" path in updateProxyConfig behind isAuthBypassed: returns 403 Forbidden if the request was not genuinely authenticated, while the rest of the management API remains accessible under the default-open posture.
  • The pre-existing empty-URL 400 validation intentionally runs before the auth gate so a malformed request does not leak whether dashboard auth is configured.
  • Added OpenAPI documentation for the new 403 response on PUT /proxy-config, including an updated endpoint description explaining the restriction.
  • Added config_proxy_ssrf_test.go with two tests: one proving an auth-bypassed caller cannot set the proxy URL, and one proving the empty-URL 400 still fires first for an invalid request.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./transports/bifrost-http/handlers/... -run TestUpdateProxyConfig

Expected outcomes:

  • TestUpdateProxyConfig_UnauthenticatedWriteRejectedPUT /proxy-config with enabled: true and a URL while BifrostContextKeyAuthBypassed is set returns 403.
  • TestUpdateProxyConfig_EmptyURLCheckRunsBeforeAuthGatePUT /proxy-config with enabled: true and an empty URL while BifrostContextKeyAuthBypassed is set returns 400, not 403.

To verify end-to-end: disable dashboard authentication, then attempt to PUT /proxy-config with a valid proxy URL. The request should be rejected with 403. Disabling the proxy (setting enabled: false) should remain unrestricted.

Breaking changes

  • Yes
  • No

Security considerations

This closes an SSRF-adjacent privilege escalation: without dashboard authentication configured, any network-reachable caller could previously redirect all outbound provider traffic — including API keys — through an arbitrary host. The fix ensures that enabling the outbound proxy always requires a genuinely authenticated admin session, independent of the global fail-open posture applied to the rest of the management API.

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

jeremym-tanium and others added 9 commits July 31, 2026 00:08
…iew maintenance (#5693)

* feat: support matview_refresh_interval "off" to disable logstore matview maintenance

The materialized views back only the dashboard UI. Deployments that run
Bifrost headless behind their own observability stack pay the REFRESH
MATERIALIZED VIEW CONCURRENTLY cost for views nothing reads, and the 5s
floor means the interval alone cannot turn maintenance off.

With "off" (or a non-positive duration) the logs store skips view
creation, the initial refresh, and the periodic refresher entirely.
matViewsReady stays false, so dashboard queries fall back to the raw
tables, and the runtime self-heal path cannot re-arm maintenance since
it only triggers from matview-path queries.

* fix: guard matview self-heal when maintenance is disabled

Review follow-up: carry the resolved disabled state onto the store so
triggerMatViewSelfHeal cannot recreate views the configuration says must
not exist, and make the schema/docs explicit that a zero duration also
disables (positive sub-5s values still clamp up).
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved token parameter compatibility handling to preserve alternative formats when the primary option is unsupported.

* **Chores**
  * Version updated to 2.0.0.
  * Enhanced load testing configuration for more reliable builds.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary

Briefly explain the purpose of this PR and the problem it solves.

## Changes

- What was changed and why
- Any notable design decisions or trade-offs

## Type of change

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

## Affected areas

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

## How to test

Describe the steps to validate this change. Include commands and expected outcomes.

```sh
# Core/Transports
go version
go test ./...

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

If adding new configs or environment variables, document them here.

## Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

## Breaking changes

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

## Related issues

Link related issues and discussions. Example: Closes #123

## Security considerations

Note any security implications (auth, secrets, PII, sandboxing, etc.).

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

Adds a `THIRD_PARTY_NOTICES.md` file to formally document third-party components used in Bifrost that carry license terms requiring explicit attribution — specifically MPL-2.0 licensed dependencies and embedded source code derived from external projects.

## Changes

- Introduces `THIRD_PARTY_NOTICES.md` to attribute:
  - Embedded source code in `framework/migrator/migrator.go` derived from `go-gormigrate/gormigrate` (MIT)
  - Go binary dependencies carrying MPL-2.0 terms: `github.com/cyphar/filepath-securejoin` and `github.com/hashicorp/go-version`
  - npm build-time devDependencies carrying MPL-2.0 terms: `lightningcss` (never shipped to end users) and `dompurify` (Apache-2.0 option elected)
- All MPL-2.0 components are used unmodified and combined as a "Larger Work" per MPL-2.0 Section 3.3; no Bifrost source files are themselves MPL-licensed.

## Type of change

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

## Affected areas

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

## How to test

No functional changes — review the file contents to confirm accuracy of license attributions against the listed upstream repositories.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

This change has no security implications. It is a legal/compliance attribution document only.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
## Summary

Briefly explain the purpose of this PR and the problem it solves.

## Changes

- What was changed and why
- Any notable design decisions or trade-offs

## Type of change

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

## Affected areas

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

## How to test

Describe the steps to validate this change. Include commands and expected outcomes.

```sh
# Core/Transports
go version
go test ./...

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

If adding new configs or environment variables, document them here.

## Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

## Breaking changes

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

## Related issues

Link related issues and discussions. Example: Closes #123

## Security considerations

Note any security implications (auth, secrets, PII, sandboxing, etc.).

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

Briefly explain the purpose of this PR and the problem it solves.

## Changes

- What was changed and why
- Any notable design decisions or trade-offs

## Type of change

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

## Affected areas

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

## How to test

Describe the steps to validate this change. Include commands and expected outcomes.

```sh
# Core/Transports
go version
go test ./...

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

If adding new configs or environment variables, document them here.

## Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

## Breaking changes

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

## Related issues

Link related issues and discussions. Example: Closes #123

## Security considerations

Note any security implications (auth, secrets, PII, sandboxing, etc.).

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

Briefly explain the purpose of this PR and the problem it solves.

## Changes

- What was changed and why
- Any notable design decisions or trade-offs

## Type of change

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

## Affected areas

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

## How to test

Describe the steps to validate this change. Include commands and expected outcomes.

```sh
# Core/Transports
go version
go test ./...

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

If adding new configs or environment variables, document them here.

## Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

## Breaking changes

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

## Related issues

Link related issues and discussions. Example: Closes #123

## Security considerations

Note any security implications (auth, secrets, PII, sandboxing, etc.).

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

Bumps several Go dependencies to their latest patch/minor versions across all modules in the repository.

## Changes

- `github.com/aws/aws-sdk-go-v2/service/s3`: `v1.97.3` → `v1.99.0`
- `github.com/aws/aws-sdk-go-v2/config`: `v1.32.11` → `v1.32.14`
- `github.com/aws/aws-sdk-go-v2/internal/ini`: `v1.8.5` → `v1.8.6`
- `github.com/weaviate/weaviate`: `v1.36.5` → `v1.38.0`
- `github.com/buger/jsonparser`: `v1.1.2` → `v1.2.0`
- `github.com/go-openapi/spec`: `v0.22.2` → `v0.22.3`
- `github.com/google/cel-go`: `v0.28.1` → `v0.29.0`
- `github.com/stretchr/objx`: `v0.5.3` added as an indirect dependency

## 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
go test ./...
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None. All changes are dependency version bumps with no security-sensitive modifications.

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

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Proxy URL updates now require authenticated admin access when dashboard authentication is disabled or bypassed.
    • Added protection against unauthenticated requests configuring attacker-controlled proxy endpoints.
    • Empty proxy URLs are rejected with a clear validation error.
  • Documentation

    • Updated API documentation to describe authentication requirements and possible forbidden responses for proxy configuration updates.

Walkthrough

The authentication middleware now marks bypassed requests. Proxy configuration rejects enabled proxy URLs from those requests with HTTP 403. Tests cover rejection and validation precedence. OpenAPI documentation describes the authentication requirement and response.

Changes

Proxy configuration authentication

Layer / File(s) Summary
Authentication bypass context
core/schemas/bifrost.go, transports/bifrost-http/handlers/middlewares.go
Adds BifrostContextKeyAuthBypassed and records it when authentication is disabled or unconfigured.
Proxy URL enforcement and validation
transports/bifrost-http/handlers/config.go, transports/bifrost-http/handlers/config_proxy_ssrf_test.go
Rejects enabled proxy URL updates from bypassed requests with HTTP 403. Tests preserve empty-URL validation with HTTP 400.
OpenAPI contract
docs/openapi/paths/management/config.yaml
Documents the authenticated access requirement and the HTTP 403 response.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant AuthMiddleware
  participant updateProxyConfig
  participant HTTPResponse
  Client->>AuthMiddleware: Send proxy configuration request
  AuthMiddleware->>updateProxyConfig: Pass request with bypass context
  updateProxyConfig->>HTTPResponse: Return 403 for enabled proxy URL
Loading

Possibly related PRs

Suggested reviewers: madhuvod, pratham-mishra04, tejasghatte

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changes address proxy authentication, not the linked issue's requested Files API support for providers. Implement the requested Files API support, or link the PR to an issue that covers the proxy authentication fix.
Out of Scope Changes check ⚠️ Warning The proxy authentication and SSRF protections are unrelated to the linked issue's Files API objectives. Update the linked issue to match this security fix, or remove the unrelated Files API issue link.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly identifies the main change: blocking unauthenticated proxy configuration.
Description check ✅ Passed The description explains the security issue, implementation, tests, affected areas, documentation, and security considerations.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch block-unauthenticated-proxy-config-writes

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

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@akshaydeo
akshaydeo marked this pull request as ready for review August 2, 2026 17:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@transports/bifrost-http/handlers/config_proxy_ssrf_test.go`:
- Line 1: Rename the test file from config_proxy_ssrf_test.go to
configproxyssrf_test.go, preserving its package and contents while removing
underscores before the required _test.go suffix.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e2dea860-38e2-4c09-9f9c-a08aa7beef0f

📥 Commits

Reviewing files that changed from the base of the PR and between cc45872 and c5668ae.

📒 Files selected for processing (5)
  • core/schemas/bifrost.go
  • docs/openapi/paths/management/config.yaml
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/config_proxy_ssrf_test.go
  • transports/bifrost-http/handlers/middlewares.go

@@ -0,0 +1,67 @@
package handlers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Rename the test file to meet the Go filename rule.

config_proxy_ssrf_test.go contains underscores before its required _test.go suffix. Rename it to configproxyssrf_test.go.

As per coding guidelines, “Go filenames must contain no underscores except the _test.go suffix.” Based on learnings, the underscore exception applies only to non-test Go files.

🤖 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 `@transports/bifrost-http/handlers/config_proxy_ssrf_test.go` at line 1, Rename
the test file from config_proxy_ssrf_test.go to configproxyssrf_test.go,
preserving its package and contents while removing underscores before the
required _test.go suffix.

Sources: Coding guidelines, Learnings

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