Ethan-Xingyue opened a new issue, #1158:
URL: https://github.com/apache/incubator-seata-go/issues/1158

   
   | Field | Value |
   | --- | --- |
   | Issue title | `[BUG] [RM] Unknown BranchType in a branch request panics 
the client instead of returning a failure response` |
   | Labels (exist in repo) | `bug`, `module/rm`, `remoting` |
   | Suggested priority | P1 (network-boundary robustness) |
   | Related | none known |
   | Verification | Reproduced 2026-09-02 on master `3bf73586` with go1.24.3 
darwin/arm64 |
   | Before publishing | Same consideration as RM-002: if an untrusted peer can 
send branch requests in your threat model, go through [email protected] 
first. |
   
   ---
   
   ## 🚀 Go Version
   
   go1.24.3 darwin/arm64
   
   ## 📦 Seata-go Version
   
   master, commit 3bf73586af81db1bd428982c93d82000d80cb1c8 (fetched 2026-09-02)
   
   ## 💾 Operating System
   
   macOS
   
   ## 📝 Bug Description
   
   `ResourceManagerCache.GetResourceManager` panics when the requested 
`BranchType` has no registered resource manager:
   
   
https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/rm/rm_cache.go#L55-L60
   
   The branch commit / rollback processors pass the `BranchType` taken from the 
incoming request straight into this lookup without any allowlist check (for 
example `rm_branch_commit_processor.go` L74 and L131, 
`rm_branch_rollback_processor.go` L76 and L130), and neither the Getty nor the 
gRPC listener converts the panic into a failure response. An unexpected enum 
value in one message therefore crashes the RM client.
   
   ## 🔄 Steps to Reproduce
   
   1. Check out the commit and create a throwaway module that points at the 
local checkout:
   
   ```bash
   git clone https://github.com/apache/incubator-seata-go.git
   cd incubator-seata-go
   git checkout 3bf73586af81db1bd428982c93d82000d80cb1c8
   repo_root="$(pwd)"
   repro_dir="$(mktemp -d)"
   cd "$repro_dir"
   go mod init seata-repro
   go mod edit -require=seata.apache.org/seata-go/[email protected]
   go mod edit -replace=seata.apache.org/seata-go/v2="$repo_root"
   ```
   
   2. Save the following as `repro_test.go` in `$repro_dir`:
   
   ```go
   package repro
   
   import (
        "testing"
   
        "seata.apache.org/seata-go/v2/pkg/protocol/branch"
        "seata.apache.org/seata-go/v2/pkg/rm"
   )
   
   func TestUnknownBranchTypeReturnsErrorInsteadOfPanicking(t *testing.T) {
        defer func() {
                if r := recover(); r != nil {
                        t.Fatalf("unknown branch type panicked: %v", r)
                }
        }()
        if got := 
rm.GetRmCacheInstance().GetResourceManager(branch.BranchType(99)); got != nil {
                t.Fatalf("unexpected resource manager: %T", got)
        }
   }
   ```
   
   3. Run:
   
   ```bash
   go mod tidy
   go test -run '^TestUnknownBranchTypeReturnsErrorInsteadOfPanicking$' 
-count=1 -v
   ```
   
   ## ✅ Expected Behavior
   
   An unknown enum value does not panic. The cache lookup reports "not found" 
through its return values, and the processor replies to the TC with a failed 
response that keeps the xid and branch id for diagnosis.
   
   ## ❌ Actual Behavior
   
   ```text
   === RUN   TestUnknownBranchTypeReturnsErrorInsteadOfPanicking
       repro_test.go:13: unknown branch type panicked: No ResourceManagerCache 
for BranchType: 99
   --- FAIL: TestUnknownBranchTypeReturnsErrorInsteadOfPanicking (0.00s)
   FAIL
   FAIL seata-audit-repros/rm003        0.616s
   FAIL
   ```
   
   ## 💡 Possible Solution
   
   - Change the lookup to `(ResourceManager, bool)` or `(ResourceManager, 
error)` and update the callers.
   - Validate the request body type and the `BranchType` against an allowlist 
in the Getty/gRPC processors before the lookup, and send a deterministic 
failure response otherwise.
   - Add panic containment at the listener level as a last line of defence, 
keeping the stack trace and a low-cardinality metric.
   
   Acceptance criteria:
   
   - [ ] The test above passes and asserts a typed not-found result.
   - [ ] Getty and gRPC commit, rollback and undo-log-delete paths each have an 
unknown-enum test.
   - [ ] The TC receives a failed response instead of waiting for a timeout; 
the client process stays alive.
   - [ ] Fuzz coverage includes negative values, max int32 and protobuf unknown 
enum values.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to