Skip to content

Restrict /logger API#7527

Merged
ffuugoo merged 4 commits intodevfrom
restrict-logger-api
Nov 13, 2025
Merged

Restrict /logger API#7527
ffuugoo merged 4 commits intodevfrom
restrict-logger-api

Conversation

@ffuugoo
Copy link
Contributor

@ffuugoo ffuugoo commented Nov 13, 2025

Fixes vulnerability exposed by /logger API. 😔

  • check access token when accessing /logger API
    • updating logger configuration requires manage access
  • on-disk log file path can only be set in config file, it can't be updated through /logger API

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you formatted your code locally using cargo +nightly fmt --all command prior to submission?
  3. Have you checked your code using cargo clippy --all --all-features command?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

- check access token
- updating logger configuration requires `manage` access
- on-disk log file path can only be set in config file,
  it can't be updated through `/logger` API
@ffuugoo ffuugoo requested a review from timvisee November 13, 2025 11:17
@coderabbitai

This comment was marked as resolved.

@timvisee timvisee added the release:1.16.0 Pull requests that should be merged for the Qdrant 1.16.0 release. label Nov 13, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4085c79 and 0197ab6.

📒 Files selected for processing (1)
  • tests/consensus_tests/auth_tests/test_jwt_access.py (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: integration-tests-consensus
  • GitHub Check: e2e-tests
  • GitHub Check: rust-tests (macos-latest)
  • GitHub Check: test-consistency
  • GitHub Check: test-shard-snapshot-api-s3-minio
  • GitHub Check: rust-tests (windows-latest)
  • GitHub Check: rust-tests-no-rocksdb (ubuntu-latest)
  • GitHub Check: integration-tests
  • GitHub Check: rust-tests (ubuntu-latest)
  • GitHub Check: lint
🔇 Additional comments (2)
tests/consensus_tests/auth_tests/test_jwt_access.py (2)

570-570: LGTM!

The access definition correctly restricts logger configuration updates to manage tokens only, aligning with the PR's security objectives.


1871-1875: LGTM!

The test implementations correctly follow the established pattern for testing endpoint access control. They will properly validate the authorization once the access definition for get_logger_config is corrected.

@ffuugoo ffuugoo requested a review from generall November 13, 2025 12:21
@ffuugoo ffuugoo merged commit 190f1cd into dev Nov 13, 2025
15 checks passed
@ffuugoo ffuugoo deleted the restrict-logger-api branch November 13, 2025 15:21
timvisee pushed a commit that referenced this pull request Nov 14, 2025
@timvisee timvisee mentioned this pull request Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:1.16.0 Pull requests that should be merged for the Qdrant 1.16.0 release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants