GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — [connectors] Optionally refuse local file URIs by swanandx · Pull Request #7737 · feldera/feldera

Conversation

swanandx commented Oct 30, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

With FELDERA_REFUSE_LOCAL_FILE_URIS=true, Delta and Iceberg connectors refuse a local path (file://... or /path). The check runs after secrets and env vars are resolved.

flowchart LR
    A[connector config] --> B["resolve ${secret} / ${env}"] --> C{local path?}
    C -- yes --> D[refuse]
    C -- no --> E[start connector]

Not covered: a local path returned by a Unity or Iceberg catalog.

Linked Issue

None.

Describe Manual Test Plan

Unit tests only. Each fails when its check is removed. Not yet tested on a cluster.

Checklist

Unit tests added/updated
Integration tests added/updated
Documentation updated
Changelog updated

Breaking Changes?

Mark if you think the answer is yes for any of these components:

OpenAPI / REST HTTP API / feldera-types / manager (What is a breaking change?)
Feldera SQL (Syntax, Semantics)
feldera-sqllib (incl. dependencies fxp, etc.) (What is a breaking change?)
Python SDK (What is a breaking change?)
fda (CLI arguments)
Adapters (including configuration)
Storage Format / Checkpoints
Others (specify)

Describe Incompatible Changes

None. The default does not change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

swanandx requested a review from gz

October 30, 2026 13:05

mythical-fred-oss Bot reviewed

Oct 30, 2026

View reviewed changes

Comment thread

crates/feldera-types/src/config.rs

		/// The local paths of a Delta Lake or Iceberg connector, as (property, value) pairs.
		/// Before references are `resolved`, skips them. After, checks every value, because a
		/// resolved value can look like a reference.
		pub fn local_file_uris(&self, resolved: bool) -> Vec<(&'static str, &str)> {

mythical-fred-oss Bot Oct 30, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tech-debt (medium): the only production caller passes resolved = true. The false branch, which skips anything that parses as a reference, is reached only from tests. That makes it a public API that can return a false "no local paths" for ${env:X}. If no pre-resolution caller is planned (for example, validation in the manager), drop the parameter and always check every value.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

swanandx Oct 30, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept: the enterprise Kubernetes runner calls this with false before secrets are resolved, so it can refuse the pipeline before it starts.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mythical-fred-oss Bot reviewed

Oct 30, 2026

View reviewed changes

Comment thread

crates/connectors/src/controller.rs Outdated

		}

		fn local_file_uris_refused() -> bool {
		std::env::var(feldera_types::config::REFUSE_LOCAL_FILE_URIS_ENV).is_ok_and(\|v\| v == "true")

mythical-fred-oss Bot Oct 30, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing (medium): no test covers this toggle or the add_input_endpoint/add_output_endpoint wiring. Both tests call resolve_connector_config with refuse_local hard-coded. Nothing fails if the env var name is wrong, if a value such as "TRUE" or "1" is silently ignored (only "true" turns the check on), or if a call site goes back to calling resolve_secret_references_in_connector_config directly. Suggestion: pass the flag in from the controller config, or add one test that starts a controller with a file:// delta output and expects invalid_transport_configuration. Also consider rejecting values that are neither true nor false, instead of failing open.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

swanandx Oct 30, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the fail-open: only unset, empty, false or 0 allow local paths now, with a test. No end-to-end test: the env var is process-wide and would break the local Delta tests that run in the same process. The cluster test of the runner change covers the wiring.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mythical-fred-oss Bot commented Oct 30, 2026

Copy link

Copy Markdown

Contributor

What I ran

Command	Result
`cargo test --locked -p feldera-types local_file_uri`	✅ pass
`cargo test --locked -p dbsp_connectors --lib local_file_uri`	✅ pass
Extra QA test, not committed: `glue.warehouse=/w` is refused and `rest.warehouse=/r` is not checked; `c:\t` and `1s3://b` are refused; `s3a://` and `memory:///` are allowed	✅ behaves as described

FELDERA_* is reserved by validate_pipeline_env, so a user cannot turn the check off through the runtime config env. Good.

Findings

pitfall (medium): the changelog and the Helm reference document pipeline.allowLocalFileUris, but the runner and chart code that sets FELDERA_REFUSE_LOCAL_FILE_URIS is not in this PR, and the PR says it is not yet tested on a cluster. Merge the docs only when the Helm value actually works. Otherwise operators may set it and believe they are protected.
missing (medium): see the inline comment. The env-var toggle and the endpoint wiring are not tested end to end.
observation: rest.warehouse is not checked while glue.warehouse is. That is probably fine, because a REST warehouse is often a catalog name, but the docs table could say so.

No hard gate fails: tests exist, nothing is flaky, there is no unsafe code, no new dependencies, and the docs are updated. The connector SQL-type gate does not apply, because this is config validation, not encoding.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

swanandx force-pushed the refuse-local-file-uris branch from 5fa26f4 to 3a55908 Compare

October 30, 2026 13:25

mythical-fred-oss Bot commented Oct 30, 2026

Copy link

Copy Markdown

Contributor

fred-review did not finish; see https://github.com/feldera/feldera/actions/runs/41063206240

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gz approved these changes

Oct 30, 2026

View reviewed changes

swanandx added this pull request to the merge queue

Oct 30, 2026

swanandx removed this pull request from the merge queue due to a manual request

Oct 30, 2026


          [connectors] Optionally refuse local file URIs

30712c2

Delta and Iceberg connectors can read pod files through file:// or a
path without a scheme. This bypasses pipeline.deniedConnectors.

With FELDERA_REFUSE_LOCAL_FILE_URIS=true, refuse such a path in the
connector config, after secrets and env vars are resolved.

Signed-off-by: Swanand Mulay <[email protected]>

swanandx force-pushed the refuse-local-file-uris branch from 3a55908 to 30712c2 Compare

October 30, 2026 15:27

swanandx added this pull request to the merge queue

Oct 30, 2026

Merged via the queue into main with commit 5de43b8

Oct 30, 2026

1 check passed

Uh oh!

There was an error while loading. Please reload this page.

swanandx deleted the refuse-local-file-uris branch

October 30, 2026 18:28

Reviewers

mythical-fred-oss[bot]

mythical-fred-oss[bot] left review comments

gz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

swanandx commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linked Issue

Describe Manual Test Plan

Checklist

Breaking Changes?

Describe Incompatible Changes

Uh oh!

mythical-fred-oss Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

swanandx Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

mythical-fred-oss Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

swanandx Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

mythical-fred-oss Bot commented Oct 30, 2026

Uh oh!

mythical-fred-oss Bot commented Oct 30, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

swanandx commented Oct 30, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.