Repository navigation
Conversation
### What problem does this PR solve? Issue Number: N/A Problem Summary: Add the startup-only enable_local_tvf setting, defaulting to false. Reject LOCAL TVF before property parsing or backend access. Explicit opt-in restores each branch's existing behavior. Dedicated regression deployments opt in to retain LOCAL coverage. ### Release note LOCAL TVF are disabled by default. Set enable_local_tvf=true in fe.conf on every query-serving FE and restart those FEs to restore the existing behavior. ### Check List (For Author) - Test: 3 focused FE unit tests passed with zero failures or skips; focused FE Checkstyle passed. SQL regression coverage added; runtime validation recorded separately. - Behavior changed: Yes. Local TVF access requires explicit startup opt-in. - Does this need documentation: Yes. Release note documents the default and opt-in.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69526 Problem Summary: Canonical glob results were not checked against the secure root. Validate canonical local paths against a nonempty canonical secure root, reject remote schemes in the shared local resolver, and enforce ADMIN/OPERATOR before local file access and during privilege checking. Keep the existing default-disabled startup gate. Give the existing glob fixture an explicit secure root so profile-specific test directories do not change its assumptions. ### Release note Enabled LOCAL reads remain restricted to user_files_secure_path and require ADMIN/OPERATOR privileges. ### Check List (For Author) - Test: 6 focused FE tests and 4 focused BE tests passed with zero failures/skips; Java Checkstyle and changed-line clang-format 16 passed. BE tests used the supported Clang16 ASAN profile. - Behavior changed: Yes, canonical secure-root containment and local authorization after explicit opt-in. - Does this need documentation: Covered by the PR release note.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69526 Problem Summary: A rejected glob result could disclose its expanded filename through a canonical-path error or a failed resolution of a dangling symlink. Return one generic target-validation error for both cases before the status reaches SQL clients. Extend temporary-directory tests to assert that neither real target names nor paths appear in errors. ### Release note LOCAL path validation errors no longer reveal expanded or resolved target filenames. ### Check List (For Author) - Test: 4 focused BE tests passed with zero failures/skips, including external wildcard, dangling symlink and error redaction assertions. Modified lines pass clang-format 16; independent Standards and Spec review found no remaining issue in this supplement. - Behavior changed: Yes, target-validation errors use a generic message. - Does this need documentation: Covered by the PR release note.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: #69526
Problem Summary: Glob results were not anchored to the canonical secure directory. Require explicit startup opt-in and enforce canonical path containment even when local TVF use is enabled.
Summary
Keep the startup-only
enable_local_tvf=falsegate before parameter parsing, backend selection and file access.Resolve both the secure root and every glob result to canonical paths, then check component containment. Reject an empty secure root and local-path requests with remote schemes.
Enforce ADMIN/OPERATOR before local file access and again during privilege checking.
Keep target-validation errors free of wildcard-expanded or resolved target names, including dangling symlinks.
Dedicated regression deployments explicitly opt in to retain LOCAL coverage.
Release note
LOCAL TVF is disabled by default. To enable it, set
enable_local_tvf=trueinfe.confon every query-serving FE and restart those FEs. The setting is not runtime-mutable. Enabled local reads require ADMIN/OPERATOR and remain restricted touser_files_secure_path.Target-validation errors no longer include expanded or resolved target filenames.
Evidence
Before: Local reads could proceed without startup opt-in, and canonical glob results were not checked against a nonempty secure root.
After: Disabled calls stop before file-service interaction; enabled calls apply authorization and canonical containment.
6 focused FE tests passed with no failures or skips.
4 focused BE tests passed with no failures or skips. The BE run used the supported Clang16 ASAN profile.
Changed C++ lines pass clang-format 16; relevant Java Checkstyle passes.
SQL regression coverage is included; the full Groovy regression runner and a live remote S3 Lance read were not run locally.
Supplemental redaction assertions cover outside wildcard matches and dangling symlinks. The 4 focused BE tests were rerun and passed with no failures or skips.
Check List (For Author)
Merge Danger
Door: Two-way through configuration and a binary rollback; opt-in retains the new path restrictions.
Blast Radius: LOCAL TVF consumers. Existing deployments must explicitly opt in, and paths outside the configured secure directory will be rejected.
Check List (For Reviewer who merge this PR)