security(providers): govern the provider role by the unified allow/block + stop the DISCOVERY_ERRORS leak#295
Merged
Conversation
…ock + stop the DISCOVERY_ERRORS leak A re-assessment found the highest-stakes role (providers — emits cloud DDL/IaC) was the one role NOT governed by the unified plugin manager: FLUID_PLUGINS_BLOCKLIST could not block a malicious provider, and provider discovery leaked the raw exception text + a FULL traceback into DISCOVERY_ERRORS — a user-facing surface (registry_dump / `fluid providers --debug`) — the exact leak shape the other hardened paths avoid. - `_discover_entrypoints` now gates each provider entry-point through `plugin_manager.is_allowed(ep.name)`, so the SAME operator allow/block policy (FLUID_PLUGINS_ALLOWLIST / FLUID_PLUGINS_BLOCKLIST) that governs validators / catalog / iac plugins also governs providers. - `_add_discovery_error` records the exception TYPE only — no raw message, no traceback (the full redaction-filtered detail still reaches the DEBUG logs). - The provider_entrypoint_failed log is type-only too; drops the now-unused `traceback` import. 422 provider tests pass; 4 new (blocklist blocks a provider, allowlist excludes, DISCOVERY_ERRORS is type-only with no secret/traceback).
📄 Documentation ReminderThis PR appears to be missing a documentation reference. Our docs live in a separate repo. Please update the PR description with one of:
See the Contributing Guide for details. |
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 & why
Follow-up from an adversarial re-assessment of the world-class push: the provider
role — the highest-stakes one (it emits cloud DDL / IaC) — was the only role NOT
governed by the unified plugin manager.
FLUID_PLUGINS_BLOCKLISTcould not block amalicious provider, and provider discovery leaked the raw exception text + a full
traceback into
DISCOVERY_ERRORS, a user-facing surface (registry_dump()/fluid providers --debug) — the exact leak shape the other hardened paths avoid.Changes
_discover_entrypointsgates each provider entry-point throughplugin_manager.is_allowed(ep.name)beforeep.load(), so the same operatorallow/block policy (
FLUID_PLUGINS_ALLOWLIST/FLUID_PLUGINS_BLOCKLIST) thatgoverns validators / catalog / iac plugins now governs providers too.
_add_discovery_errorrecords the exception type only — no raw message, notraceback (the full redaction-filtered detail still reaches DEBUG logs); drops the
now-unused
tracebackimport; the failure log is type-only too.Built-ins are unaffected — they self-register via the curated/subpackage paths
regardless of the entry-point gate (verified).
Verification
allowlist excludes, DISCOVERY_ERRORS type-only with no secret/traceback).
ruff+black==24.10.0clean.bypass; gate runs before
load(); built-ins safe; no import cycle.fluid providerswith an out-of-tree provider: discovered bydefault;
FLUID_PLUGINS_BLOCKLIST=acmeprov→ blocked (previously impossible).