[OIiveirra]

What problem does this PR solve?

Issue Number: N/A

Related PR: #69526

Problem Summary:

Glob expansion could return paths outside user_files_secure_path because
the expanded targets were not checked against a nonempty canonical
secure root. LOCAL Lance reads also bypassed local path validation
during schema discovery and scanning.

• Disable LOCAL TVF and FILE TVF with local storage by default using the
  startup-only enable_local_tvf setting.
• Check ADMIN/OPERATOR privileges before property parsing, backend
  selection and file discovery, and repeat the check during
  authorization. FILE with local storage delegates to the same guarded
  entry point.
• Canonicalize the secure root and each expanded glob target before
  checking directory containment. Reject empty secure roots, remote
  schemes and invalid local paths. Return generic errors for failed or
  out-of-root target resolution, including dangling symlinks.
• Validate LOCAL Lance paths before both schema discovery and dataset
  scanning. Resolve relative paths under the BE secure root and preserve
  the existing fixed-version local catalog paths.
• Retain LOCAL/S3 file types in Lance scan ranges. Validate the final S3
  Lance dataset URI before metadata loading, independently of the chosen
  storage-property type, including Broker passthrough. Reject local URI
  aliases and malformed authorities that the SDK could treat as local
  paths. Preserve supported remote providers, hostless GooseFS and the
  existing normalization of S3/OSS HTTP URLs.
• Explicitly enable LOCAL TVF in existing regression deployments to
  retain their LOCAL test coverage.

Release note

LOCAL TVF and FILE TVF with local storage are disabled by default. To
use them, set enable_local_tvf=true in fe.conf on every query-serving FE
and restart those FEs. The setting cannot be changed at runtime. Enabled
reads still require ADMIN/OPERATOR and must remain within the configured
BE user_files_secure_path. Errors no longer disclose expanded or
resolved target names. S3 Lance accepts only valid supported remote
dataset URIs, including when storage properties pass a URI through.

Check List (For Author)

• Test
  • Regression test (added; runtime validation incomplete)
  • Unit Test
  • Manual test (add detailed scripts or steps below)
  • No need to test or manual test. Explain why:
    • This is a refactor/code format and no logic has been
      changed.
    • Previous test can cover this change.
    • No code files have been changed.
    • Other reason

Local validation:

• 31 focused FE unit tests passed with fresh XML, zero failures/errors/
  skips and unchanged sources during the run. The selected classes cover
  the LOCAL startup gate/authorization, scan-range identity and S3 Lance
  URI validation. New tests reject Broker local paths/aliases before any
  metadata-loader call and confirm supported remote URIs, valid ports,
  underscore buckets, hostless GooseFS and S3/OSS HTTP normalization
  reach the mocked loader. The focused Broker file-URI regression failed
  before the fix and passes afterward. Remote data reads are not claimed
  here.

• 10 selected BE tests previously passed under Clang21/ASAN with fresh
  XML, zero failures/skips and unchanged sources during the run. They
  cover secure glob/path checks, real Lance schema retrieval, an actual
  relative LOCAL scan returning row IDs {1,2,3,4}, fixed-version local
  catalog routing and S3 URI/options routing.

• 34 real SQL checks passed on a private FE before this URI supplement.
  They cover default-disable/restart opt-in, immutable startup
  configuration, ADMIN/ordinary users, LOCAL/FILE and SELECT/EXPLAIN.
  This FE-only run validates policy and rejection paths, with no
  registered BE.

• The BE build uses task-private dependencies: branch-pinned Lance-C and
  Paimon rebuilt with Rust 1.94.0. Repository dependency files and
  product code were not changed for testing.

• Relevant Java Checkstyle and git diff --check passed for this
  supplement. The BE implementation is unchanged since the earlier
  BE/style checks. Full Groovy regression and actual remote S3 Lance
  data reads remain unverified locally.

• Behavior changed:

  • No.
  • Yes. LOCAL and FILE-local require explicit startup opt-in;
    enabled reads enforce authorization and secure-root containment.
    S3 Lance also validates the final remote dataset URI before opening.

• Does this need documentation?

  • No.
  • Yes. Document enable_local_tvf and the LOCAL/FILE-local
    upgrade behavior. Documentation PR pending; the release note
    above does not replace the user documentation.

Check List (For Reviewer who merge this PR)

• Confirm the release note
• Confirm test cases
• Confirm document
• Add branch pick label

[hello-stephen]

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:

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

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[OIiveirra]

run buildall

[OIiveirra]

/review

[github-actions]

Two verified findings require changes: [P1] signed-zero port syntax still lets S3 Lance bypass the new LOCAL admission policy through the SDK's local-path fallback; [P2] LOCAL Lance file-URI canonicalization changes percent-encoded dataset identity. Both are reported inline. There were no existing inline threads or blocking comment IDs, and no additional user-provided focus points.

All 20 changed files were reviewed, with FE, BE and independent boundary-risk coverage. The complete LOCAL/FILE/S3 entrypoint -> property normalization -> schema/metadata discovery -> scan-range -> dataset-open/cache chain was traced against this head and the pinned SDK sources.

Individual critical-checkpoint conclusions:

1. Goal and proof: default-disable, early/repeated privileges and canonical glob containment are implemented, but the two findings leave security and URI compatibility incomplete. Added tests exercise the intended paths; they miss signed-zero ports and escaped file-URI collisions.
2. Scope: production edits are focused on admission, canonicalization and Lance identity, with corresponding tests and regression opt-ins.
3. Concurrency and locks: no new threads, shared mutable state or lock order; existing heavy-work RPC/scanner contexts perform filesystem work without newly introduced critical sections.
4. Lifecycle/static initialization: existing RAII, close paths, per-reader dataset ownership and lazy Session initialization remain intact; no new cross-TU static dependency or ownership cycle.
5. Configuration: enable_local_tvf defaults false and is startup-only because ConfField.mutable defaults false and setMutableConfig rejects it. Every serving FE must opt in and restart, as described.
6. Compatibility: no storage/wire format change; legacy scan-level file-type fallback and positive-version catalog behavior remain. Encoded file-URI compatibility regresses as reported.
7. Thrift/protobuf: no definitions changed, so release-branch ID comparison is not applicable.
8. Parallel paths: LOCAL and FILE-local share constructor/authorization guards; schema and execution use FILE_LOCAL/version zero; S3 and catalog snapshots use fixed positive versions. The S3 parser bypass remains actionable.
9. Conditions: range-type precedence and version-zero exceptions are documented and backed by actual producer invariants; no user TVF option supplies a positive local version.
10. Test coverage: inspected FE positive/negative admission and URI tests, BE glob/root/symlink/schema/relative-scan tests, and SELECT/EXPLAIN Groovy gate checks. Add cases for both inline findings. No build or test was run under this static-only contract.
11. Expected results: no generated output snapshots changed; the new four-row expectation matches the existing Lance fixture usage. The Groovy suite checks gate/backend rejection, not a successful end-to-end read.
12. Observability: existing profile/error paths remain; generic containment errors intentionally redact expanded/resolved target names. No new critical metrics requirement identified for admission-only work.
13. Transactions/persistence: no EditLog, transaction, visible-version, delete-bitmap or master-failover behavior changes.
14. Writes/atomicity/crashes: production changes are read-only; no new data-write transaction or persistent cleanup state.
15. FE/BE variables: an existing file_type field is now retained on Lance TVF execution ranges; schema already tags it and legacy scan-level handling is preserved.
16. Performance/memory: validation is per TVF/schema/split rather than per row; no material new allocation, cache or hot-loop regression substantiated. Glob root recanonicalization is bounded per match.
17. Further recheck: candidate triggers, upstream guarantees, downstream SDK dispatch, tests and all changed paths were rechecked; no other introduced actionable defect was verified.

Review limits: static source review only; author-reported runs were not independently executed, and actual remote reads, Groovy integration and mixed-version runtime behavior remain untested here. Pre-existing descendant-symlink/manifest child confinement and filesystem-mutation races were examined and excluded from new findings because this PR did not introduce or expand them. No unresolved review candidate or uncovered changed-file scope remains.

Review converged after 2 round(s).

[github-actions]

[P1] Reject signed-zero ports before Lance falls back to local files

HostAndPort.fromString accepts bucket:-0: Guava parses the port as integer zero. Java URI also accepts this authority, so s3("format"="lance", "fs.broker.support"="true", "uri"="s3://bucket:-0/../../dataset.lance") passes this gate. Rust's port parser rejects the minus, and Lance then falls back to a local path; lexical normalization resolves this example to <FE cwd>/dataset.lance. An ordinary user can therefore open an existing FE-local Lance dataset despite enable_local_tvf=false and without ADMIN/OPERATOR. Matching BE-local datasets also bypass containment because ranges remain FILE_S3. Reject signed port spellings, including negative zero, or validate with the SDK parser, and cover this Broker passthrough case.

[github-actions]

[P2] Decode file-URI paths before canonical containment checks

For an enabled, authorized LOCAL Lance read with user_files_secure_path=/secure, file_path=file:///secure/sample%20data.lance previously opened /secure/sample data.lance: the pinned Lance file provider uses Url::to_file_path. This new call routes the URI through convert_to_abs_path, which strips the scheme/authority without decoding %20, so canonicalization instead checks /secure/sample%20data.lance. The valid dataset now fails schema discovery; if both directory spellings contain datasets, schema discovery and scanning silently select the percent-named dataset instead. Decode file: URI paths before canonicalization while preserving literal plain-path semantics, and add escaped-space/percent collision cases to both schema and scan coverage.