[fix](fe) Reject invalid stream load tokens on commit and rollback by liaoxin01 · Pull Request #62111 · apache/doris

liaoxin01 · 2026-04-03T16:02:13Z

What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: Stream load commit and rollback accepted invalid tokens because the return value of checkToken() was ignored.

Release note

None

Check List (For Author)

Test: No completed automated test run in this environment. I attempted a FE build in a worktree and reached fe-core, but did not wait for full FE packaging to finish.
Behavior changed: Yes. Invalid token requests are now rejected on stream load commit and rollback.
Does this need documentation: No

hello-stephen · 2026-04-03T16:02:20Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

Copilot

Pull request overview

This PR addresses a security/authentication correctness issue in FE stream-load transaction handling by enforcing the return value of checkToken() during commit operations.

Changes:

Reject invalid stream-load tokens during loadTxnCommitImpl by throwing AuthenticationException when checkToken() returns false.
Reject invalid tokens during commitTxnImpl with the same checkToken() return-value enforcement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dataroaring · 2026-04-06T13:04:27Z

run buildall

dataroaring · 2026-04-06T13:04:33Z

/review

github-actions

Found 1 issue.

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java: the rollback RPCs still ignore the result of checkToken(). loadTxnRollbackImpl() (line 1966 in the current head) and rollbackTxnImpl() (line 2085) both continue to abortTransaction(...) even when the token is invalid, so the PR does not yet satisfy its stated goal of rejecting invalid tokens on commit and rollback.

Critical checkpoints:

Goal: Not fully achieved; commit paths are fixed, rollback paths remain vulnerable. No test proves both commit and rollback rejection.
Minimality: Small and focused change, but incompletely applied to the parallel rollback paths.
Concurrency: Not applicable; this patch only changes FE auth guards.
Lifecycle/static init: Not applicable.
Config: No config changes.
Compatibility: No protocol or storage compatibility impact.
Parallel code paths: Failed; equivalent rollback handlers were missed.
Special checks: The new explicit boolean check is the right pattern where added.
Test coverage: Missing coverage for invalid-token rollback and end-to-end auth rejection.
Observability: Existing RPC status conversion remains adequate.
Transaction/persistence: No EditLog or persistence changes, but auth gating for rollback is still incorrect.
Data writes/modifications: Unauthorized rollback is still possible with an invalid token.
FE-BE variable passing: No new variables.
Performance: Negligible impact.
Other issues: None beyond the missed rollback paths.

### What problem does this PR solve? Issue Number: None Related PR: apache#62111 Problem Summary: Reject invalid tokens in stream load rollback and transaction rollback paths, avoid echoing raw tokens in AuthenticationException messages, and add unit coverage for invalid-token commit and rollback requests. ### Release note Invalid tokens are now rejected consistently for commit and rollback requests without exposing the token value in error messages. ### Check List (For Author) - Test: FE unit test - ./run-fe-ut.sh --run org.apache.doris.service.FrontendServiceImplTest#testLoadTxnCommitRejectsInvalidToken+testLoadTxnRollbackRejectsInvalidToken+testCommitTxnRejectsInvalidToken+testRollbackTxnRejectsInvalidToken - Behavior changed: Yes (invalid tokens now fail consistently on rollback and error messages no longer expose token values) - Does this need documentation: No

liaoxin01 · 2026-04-08T06:37:48Z

run buildall

Issue Number: None Related PR: apache#62111 Problem Summary: Reject invalid tokens in stream load rollback and transaction rollback paths, avoid echoing raw tokens in AuthenticationException messages, and add unit coverage for invalid-token commit and rollback requests. Invalid tokens are now rejected consistently for commit and rollback requests without exposing the token value in error messages. - Test: FE unit test - ./run-fe-ut.sh --run org.apache.doris.service.FrontendServiceImplTest#testLoadTxnCommitRejectsInvalidToken+testLoadTxnRollbackRejectsInvalidToken+testCommitTxnRejectsInvalidToken+testRollbackTxnRejectsInvalidToken - Behavior changed: Yes (invalid tokens now fail consistently on rollback and error messages no longer expose token values) - Does this need documentation: No

liaoxin01 · 2026-04-10T07:58:24Z

run buildall

hello-stephen · 2026-04-10T09:19:24Z

FE UT Coverage Report

Increment line coverage 14.29% (1/7) 🎉
Increment coverage report
Complete coverage report

### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Stream load commit and rollback accepted invalid tokens because the return value of checkToken() was ignored. ### Release note None ### Check List (For Author) - Test: No need to test (small FE auth guard change; no completed automated test run in this environment) - Behavior changed: Yes (invalid token is now rejected for stream load commit/rollback) - Does this need documentation: No

Issue Number: None Related PR: apache#62111 Problem Summary: Reject invalid tokens in stream load rollback and transaction rollback paths, avoid echoing raw tokens in AuthenticationException messages, and add unit coverage for invalid-token commit and rollback requests. Invalid tokens are now rejected consistently for commit and rollback requests without exposing the token value in error messages. - Test: FE unit test - ./run-fe-ut.sh --run org.apache.doris.service.FrontendServiceImplTest#testLoadTxnCommitRejectsInvalidToken+testLoadTxnRollbackRejectsInvalidToken+testCommitTxnRejectsInvalidToken+testRollbackTxnRejectsInvalidToken - Behavior changed: Yes (invalid tokens now fail consistently on rollback and error messages no longer expose token values) - Does this need documentation: No

liaoxin01 · 2026-04-17T10:00:25Z

run buildall

liaoxin01 · 2026-04-17T10:00:44Z

/review

hello-stephen · 2026-04-17T11:24:03Z

FE UT Coverage Report

Increment line coverage 14.29% (1/7) 🎉
Increment coverage report
Complete coverage report

github-actions · 2026-04-17T11:56:53Z

OpenCode automated review failed and did not complete.

Error: Review step was failure (possibly timeout or cancelled)
Workflow run: https://github.com/apache/doris/actions/runs/24559402638

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

liaoxin01 · 2026-04-17T12:00:42Z

/review

hello-stephen · 2026-04-17T12:20:51Z

FE Regression Coverage Report

Increment line coverage 0.00% (0/7) 🎉
Increment coverage report
Complete coverage report

github-actions · 2026-04-17T13:56:48Z

OpenCode automated review failed and did not complete.

Error: Review step was failure (possibly timeout or cancelled)
Workflow run: https://github.com/apache/doris/actions/runs/24563974343

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

liaoxin01 · 2026-04-22T07:29:43Z

/review

github-actions

The token-only path is fixed, but I do not think this PR reaches its stated goal for the real commit/rollback request shapes that Doris currently sends.

Critical checkpoints:

Goal: Not fully achieved. loadTxnCommitImpl() / commitTxnImpl() / loadTxnRollbackImpl() / rollbackTxnImpl() only validate the token when auth_code is absent. Current BE callers still populate deprecated auth_code on these RPCs, so they continue to bypass the new check.
Minimality: The patch is small and focused.
Concurrency: No new concurrency or locking risk in this change.
Lifecycle / static init: Not applicable.
Config: No config changes.
Compatibility: No storage or protocol compatibility impact.
Parallel code paths: Incomplete. The legacy auth_code branch remains an authentication bypass for the same commit/rollback RPCs.
Special checks: checkTokenOrThrow() is the right local helper, but the surrounding condition still prevents it from running on current callers.
Test coverage: The new FE tests only exercise private *Impl methods with token-only requests. They do not cover the public RPC path or requests that still carry auth_code, so they do not prove the production callers are fixed.
Test result changes: Added FE unit tests; I did not rerun them locally in this review.
Observability: Existing logging / status propagation is sufficient.
Transaction / persistence: No EditLog or persistence changes.
Data writes / modifications: Unauthorized commit or rollback is still possible through the deprecated auth_code path.
FE-BE variable passing: No new variables.
Performance: No meaningful impact.
Other issues: None beyond the authentication bypass / missing coverage above.

Please either stop sending deprecated auth_code on these callers or make the FE handlers validate or reject it before treating the request as authenticated, then add coverage for the real request shape.

this is another problem, should be fixed in another pr.

github-actions · 2026-04-24T07:51:16Z

PR approved by at least one committer and no changes requested.

github-actions · 2026-04-24T07:51:19Z

PR approved by anyone and no changes requested.

… rollback (#63066) Pick #62111

…pache#62111) Problem Summary: Stream load commit and rollback accepted invalid tokens because the return value of checkToken() was ignored.

Copilot AI review requested due to automatic review settings April 3, 2026 16:02

Copilot started reviewing on behalf of liaoxin01 April 3, 2026 16:03

View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

liaoxin01 force-pushed the fix-streamload-token-auth-master branch from bd4043b to 226c408 Compare April 3, 2026 16:14

github-actions Bot reviewed Apr 6, 2026

View reviewed changes

Comment thread fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java Resolved

liaoxin01 force-pushed the fix-streamload-token-auth-master branch from dc3e309 to 061a919 Compare April 10, 2026 07:54

liaoxin01 added 2 commits April 17, 2026 17:56

liaoxin01 force-pushed the fix-streamload-token-auth-master branch from 061a919 to 85da7e3 Compare April 17, 2026 09:59

github-actions Bot previously requested changes Apr 22, 2026

View reviewed changes

Comment thread fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java Resolved

liaoxin01 added the dev/4.1.x label Apr 24, 2026

gavinchou approved these changes Apr 24, 2026

View reviewed changes

github-actions Bot added the approved Indicates a PR has been approved by one committer. label Apr 24, 2026

github-actions Bot added the reviewed label Apr 24, 2026

liaoxin01 merged commit 589a32c into apache:master Apr 24, 2026
33 checks passed

liaoxin01 deleted the fix-streamload-token-auth-master branch April 24, 2026 07:52

github-actions Bot added the dev/4.1.x-conflict label Apr 24, 2026

liaoxin01 mentioned this pull request May 8, 2026

branch-4.1: [fix](fe) Reject invalid stream load tokens on commit and rollback #63066

Merged

yiguolei pushed a commit that referenced this pull request May 9, 2026

branch-4.1: [fix](fe) Reject invalid stream load tokens on commit and…

1d80f0a

… rollback (#63066) Pick #62111

yiguolei added dev/4.1.1-merged and removed dev/4.1.x dev/4.1.x-conflict labels May 9, 2026

yiguolei mentioned this pull request May 20, 2026

4.1.1 Release Notes #63426

Closed

Conversation

liaoxin01 commented Apr 3, 2026

What problem does this PR solve?

Release note

Check List (For Author)

Uh oh!

hello-stephen commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dataroaring commented Apr 6, 2026

Uh oh!

dataroaring commented Apr 6, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

liaoxin01 commented Apr 8, 2026

Uh oh!

liaoxin01 commented Apr 10, 2026

Uh oh!

hello-stephen commented Apr 10, 2026

FE UT Coverage Report

Uh oh!

liaoxin01 commented Apr 17, 2026

Uh oh!

liaoxin01 commented Apr 17, 2026

Uh oh!

hello-stephen commented Apr 17, 2026

FE UT Coverage Report

Uh oh!

github-actions Bot commented Apr 17, 2026

Uh oh!

liaoxin01 commented Apr 17, 2026

Uh oh!

hello-stephen commented Apr 17, 2026

FE Regression Coverage Report

Uh oh!

github-actions Bot commented Apr 17, 2026

Uh oh!

liaoxin01 commented Apr 22, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants