DanielLeens commented on PR #10838:
URL: https://github.com/apache/seatunnel/pull/10838#issuecomment-4679660667

   Thanks for the thoughtful explanation. I rechecked the current head and I 
still think the blocker is real on this PR as it stands. The reason is not just 
that one endpoint would use a different auth knob. Enabling 
`metadata.enabled=true` with `kind=dynamic` currently creates a write API that 
persists cluster-wide connector credentials and later influences job execution, 
while the only safety boundary remains the optional global REST auth. On the 
current path, `JettyService.createJettyServer()` still mounts 
`DynamicMetadataDataSourceServlet`, and 
`DynamicMetadataDataSourceService.create/update/deleteDatasource(...)` can 
rewrite future datasource definitions even when `enable-basic-auth=false`. 
Documentation and startup warnings would help operators, but they do not change 
that runtime boundary on the feature-enabled path, so I still do not think 
warnings alone are enough for merge. On the masking point, I agree the docs 
should not promise a built-in secret list. But the default GET
 /list path still returns plaintext values unless operators remember to 
configure `sensitive_keys`, which is another reason I am not comfortable 
merging the management surface as-is. There is no new commit after my last 
review, so I am not doing another full code pass on this revision. If you want 
to keep the single global REST auth model, I think the safer direction is to 
move this write surface behind a stronger default boundary at the REST layer 
itself, or narrow the feature so this PR does not expose secret-bearing CRUD on 
an unprotected-by-default path. Happy to re-review once there is a concrete 
code update or a maintainer-level security decision to evaluate.


-- 
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]

Reply via email to