markhoerth commented on issue #12760: URL: https://github.com/apache/gravitino/issues/12760#issuecomment-5529549098
@jerryshao Fair question, let me restate it plainly. Jersey and all five filters are bound to `API_ANY_PATH = "/api/*"`. `addServlet` accepts any pathspec. A servlet mounted at a root path therefore gets no authentication, no audit, no versioning and no custom filters, and nothing in the build reports it. Whether an endpoint is protected is decided by the string typed at the mount site and never checked. Worth noting how the binding works. `addFilter`, `addCustomFilters` and `addSystemFilters` each take a `pathSpec` argument and are called five times with `API_ANY_PATH`. There is no default binding, so any new pathspec has to be passed at all five sites and nothing fails if one is missed. Lines 210-211 already bind `WebUIFilter` at `/` and `/ui/*`, so filters outside `/api/*` are an established pattern rather than a hypothetical. Nothing that ships in 1.3 is exposed by this. Three of the four servlets are public by intent and correct. The fourth, `SecretProvidersConfigServlet`, is on main only, and it is in the issue as evidence rather than as the thing to fix: it was written, reviewed and merged through this gap, with a javadoc arguing it was safe because it followed the `ConfigServlet` auth model. `ConfigServlet` publishes a set the operator controls through `visibleConfigs`; the secrets servlet publishes the entire provider registry with no operator control. The requirement is one test. Every path passed to `addServlet` is either under a pathspec the filter chain covers, or named in an explicit public list. Anything under `/api/*` needs nothing and is unaffected. The four existing servlets all pass. What the test prevents is the next one being added by accident and shipping. Two further reasons it is worth doing. First, an endpoint that answers without appearing in the audit log is a gap independent of what it returns, and audit is a compliance requirement for our enterprise customers. Second, I want the internal endpoints moved off `/api`. Once that happens, paths outside `/api/*` become a real surface rather than three health aliases, and each one needs its filters bound explicitly at every call site. The test is what confirms that happened rather than us assuming it. One correction to record while this is open. The contract probe that reported no unauthenticated access across 242 endpoints built its inventory from `@Path` annotations and ran entirely under `/api/*`. Servlet paths carry no `@Path` and were never in it. That result means authentication is enforced under `/api/*` and says nothing about paths outside it. -- 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]
