Skip to content

auth path whitelisting - #5765

Open
akshaydeo wants to merge 9 commits into
devfrom
align-auth-whitelist-with-router-path-matching
Open

auth path whitelisting#5765
akshaydeo wants to merge 9 commits into
devfrom
align-auth-whitelist-with-router-path-matching

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes an authentication bypass vulnerability where percent-encoded path traversal sequences (e.g. ..%2F) could be used to reach protected API endpoints without authentication. Because fasthttp/router dispatches routes using the raw, still-encoded PathOriginal(), a request like /api/providers/..%2Fskills%2Fserve%2Fmalicious is matched by the router as a single opaque segment against the protected /api/providers/{provider} route. However, the previous whitelist check used ctx.Path(), which decodes and normalizes the path first — making the same request appear to be the whitelisted /api/skills/serve/malicious prefix, allowing it to sail through unauthenticated.

Changes

  • The auth middleware whitelist check now uses ctx.Request.URI().PathOriginal() (the raw, percent-encoded path) instead of ctx.Path() (the decoded, dot-segment-collapsed path), keeping the auth decision congruent with how the router dispatches the request.
  • Added a regression test (TestAuthMiddleware_EncodedTraversalDoesNotBypassAuth) covering the exact PoC traversal pattern, variations with different whitelisted prefixes, doubly-encoded dot segments, and a sanity check confirming that legitimate unencoded whitelisted routes still bypass auth correctly.

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 TestAuthMiddleware_EncodedTraversalDoesNotBypassAuth -v
go test ./transports/bifrost-http/handlers/...

The new regression test should pass, confirming that encoded traversal requests return 401 Unauthorized and that legitimate whitelisted routes continue to bypass auth as expected.

Breaking changes

  • Yes
  • No

Security considerations

This is a security fix for an authentication bypass. Requests using percent-encoded path traversal sequences to make a protected route appear whitelisted are now correctly blocked. Any client relying on encoded traversal paths to access protected endpoints without credentials will receive 401 Unauthorized. Legitimate clients using properly formed, unencoded paths are unaffected.

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 8 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
@akshaydeo
akshaydeo marked this pull request as ready for review August 2, 2026 05:22
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 87e30ab4-8136-4d19-98dc-2bfe424744bc

📥 Commits

Reviewing files that changed from the base of the PR and between 1ca7677 and 37e58b8.

📒 Files selected for processing (2)
  • transports/bifrost-http/handlers/middlewares.go
  • transports/bifrost-http/handlers/middlewares_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • transports/bifrost-http/handlers/middlewares.go
  • transports/bifrost-http/handlers/middlewares_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved authentication checks for encoded and doubly encoded path traversal attempts.
    • Prevented unauthorized requests from accessing routes exempt from authentication.
    • Preserved access for legitimate, unencoded requests to approved routes.

Walkthrough

The authentication middleware now matches whitelist entries against PathOriginal(). Regression tests cover encoded and doubly encoded traversal paths and confirm that only the legitimate skills route bypasses authentication.

Changes

Authentication path matching

Layer / File(s) Summary
Raw path authentication matching
transports/bifrost-http/handlers/middlewares.go, transports/bifrost-http/handlers/middlewares_test.go
The middleware uses PathOriginal() for whitelist matching. Tests verify that encoded traversal requests return 401 Unauthorized without invoking the next handler, while the valid skills route remains accessible.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • maximhq/bifrost#5763: Both changes address authentication bypasses caused by path normalization.
  • maximhq/bifrost#5778: Both changes modify authentication middleware behavior, including authentication bypass handling.

Suggested reviewers: tejasghatte, pratham-mishra04, roroghost17

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main authentication path-whitelisting change.
Description check ✅ Passed The description covers the purpose, changes, testing, affected areas, security impact, breaking changes, and checklist.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 align-auth-whitelist-with-router-path-matching

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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.

@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/middlewares_test.go`:
- Around line 765-766: Update the traversal-path test case in the middleware
test table to use double percent encoding for each dot and slash component, such
as the proposed %25-prefixed form, so it exercises two decoding passes while
preserving the expected rejection behavior.
🪄 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: 392560bf-8ebd-46e0-92e2-44a764328afb

📥 Commits

Reviewing files that changed from the base of the PR and between e493a6a and 1ca7677.

📒 Files selected for processing (2)
  • transports/bifrost-http/handlers/middlewares.go
  • transports/bifrost-http/handlers/middlewares_test.go

Comment thread transports/bifrost-http/handlers/middlewares_test.go Outdated
@akshaydeo
akshaydeo force-pushed the align-auth-whitelist-with-router-path-matching branch from 1ca7677 to 37e58b8 Compare August 3, 2026 00:05
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 3, 2026
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