Skip to content

fix(receive): validate tenant label#8806

Open
guidonguido wants to merge 1 commit intothanos-io:mainfrom
guidonguido:fix/validate-tenant-label
Open

fix(receive): validate tenant label#8806
guidonguido wants to merge 1 commit intothanos-io:mainfrom
guidonguido:fix/validate-tenant-label

Conversation

@guidonguido
Copy link
Copy Markdown

@guidonguido guidonguido commented May 5, 2026

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

When --receive.split-tenant-label-name is configured, tenant IDs extracted from labels are now validated against IsTenantValid() as done for HTTP header and certificate-based tenants.

  • pkg/receive/handler.go: validate tenant IDs extracted from labels, matching the existing validation applied to HTTP header tenants.
  • test/e2e/receive_test.go: Add TestReceivePathTraversal with subtests verifying both HTTP header and
    label-based path traversal attempts are rejected.

Verification

  • all unit tests pass
  • e2e test TestReceivePathTraversal confirms labels like ../mal-path are now rejected

@guidonguido guidonguido force-pushed the fix/validate-tenant-label branch from 1043f2f to f348ea8 Compare May 5, 2026 07:24
saswatamcode
saswatamcode previously approved these changes May 5, 2026
Copy link
Copy Markdown
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

Thanks! Generally LGTM!
I wonder if we can skip e2e test for this here, and just check handler

Comment thread test/e2e/receive_test.go Outdated
Comment thread test/e2e/receive_test.go Outdated
})
}

func TestReceiveTenantValidation(t *testing.T) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe instead of e2e test, we should just have a handler test? So we can check the error as well?

Signed-off-by: Guido Ricioppo <griciopp@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants