SEZ9 commented on PR #11557: URL: https://github.com/apache/seatunnel/pull/11557#issuecomment-5390961148
Thanks for the updates, @DanielLeens. I've read through your notes on `0342ae2189f5d2ba9465fef461120158cae98d18` (the loopback narrowing for the engine-server test wiring) and `8dfbf773206` (removing `connector-allowlist` in favor of the `managed-source-runtime.enabled` flag plus `ManagedSourceCapability`, with fail-closed deployment). Both changes read as sensibly scoped, and I appreciate you keeping the conflict resolution with `dev` separate. That said, my remaining blocking ask from the earlier review is still open: - **PR11557-F1 (HIGH, Security): unsafe Java deserialization pattern** in `seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/task/operation/source/ManagedSourceRegisterOperation.java`. Based on your own delta summary (`e62beaaadf98` → `8dfbf773206e`), neither of the two new commits touches this file, so I'm treating it as unaddressed. Please either fix the deserialization path (e.g., replace the unsafe pattern with an explicit, type-constrained serialization approach) or reply here explaining why the current pattern is safe in this operation's context so we can close the finding with rationale. Concretely, before this can move forward I need: 1. A commit addressing F1 (or a written justification on this thread), and 2. The merge conflict with `dev` resolved, which you've already flagged as pending. Everything else in your two updates looks fine to me within the scope of what I previously raised. Thanks for the careful, well-documented iterations. <!-- streview-comment:519 --> -- 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]
