Skip to content

Commit 6ce33ea

Browse files
Airbyte-Supportdevin-ai-integration[bot]darynaishchenko
authored
fix(declarative): allow bundled custom components without AIRBYTE_ENABLE_UNSAFE_CODE (#1083)
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Daryna Ishchenko <darina.ishchenko17@gmail.com>
1 parent 7e96546 commit 6ce33ea

10 files changed

Lines changed: 240 additions & 83 deletions

File tree

airbyte_cdk/cli/source_declarative_manifest/_run.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@
4343
from airbyte_cdk.sources.declarative.concurrent_declarative_source import (
4444
ConcurrentDeclarativeSource,
4545
)
46+
from airbyte_cdk.sources.declarative.parsers.custom_code_compiler import (
47+
AirbyteCustomCodeNotPermittedError,
48+
custom_code_execution_permitted,
49+
)
4650
from airbyte_cdk.sources.declarative.yaml_declarative_source import YamlDeclarativeSource
4751
from airbyte_cdk.sources.source import TState
4852
from airbyte_cdk.utils.datetime_helpers import ab_datetime_now
@@ -212,6 +216,10 @@ def create_declarative_source(
212216
state=state,
213217
source_config=cast(dict[str, Any], config["__injected_declarative_manifest"]),
214218
config_path=parsed_args.config if hasattr(parsed_args, "config") else None,
219+
# The manifest is supplied by the caller (via config or --manifest-path) rather than
220+
# bundled in a connector image, so any custom components it references are untrusted
221+
# code. Bundled manifest-only connectors take the local-manifest path instead.
222+
custom_components_trusted=False,
215223
)
216224
except Exception as error:
217225
print(
@@ -275,10 +283,17 @@ def _parse_manifest_from_file(filepath: str) -> dict[str, Any] | None:
275283

276284

277285
def _register_components_from_file(filepath: str) -> None:
278-
"""Load and register components from a Python file specified in the args."""
286+
"""Load and register components from a Python file specified in the args.
287+
288+
The file is caller-supplied Python that is executed via `exec_module`, so it is
289+
untrusted code and must be gated behind `AIRBYTE_ENABLE_UNSAFE_CODE`.
290+
"""
279291
import importlib.util
280292
import sys
281293

294+
if not custom_code_execution_permitted():
295+
raise AirbyteCustomCodeNotPermittedError
296+
282297
components_path = Path(filepath)
283298

284299
module_name = "components"

airbyte_cdk/connector_builder/connector_builder_handler.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ def create_source(
8484
migrate_manifest=should_migrate_manifest(config),
8585
normalize_manifest=should_normalize_manifest(config),
8686
limits=limits,
87+
# The manifest is supplied by the Connector Builder caller rather than bundled in a
88+
# connector image, so any custom components it references are untrusted code.
89+
custom_components_trusted=False,
8790
)
8891

8992

airbyte_cdk/manifest_server/command_processor/utils.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ def build_source(
7979
state=state,
8080
source_config=definition,
8181
config=config,
82+
# The manifest is supplied by the API caller rather than bundled in a connector image, so
83+
# any custom components it references are untrusted code.
84+
custom_components_trusted=False,
8285
normalize_manifest=should_normalize,
8386
migrate_manifest=should_migrate,
8487
emit_connector_builder_messages=True,

airbyte_cdk/sources/declarative/concurrent_declarative_source.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ def __init__(
150150
normalize_manifest: bool = False,
151151
limits: Optional[TestLimits] = None,
152152
config_path: Optional[str] = None,
153+
custom_components_trusted: bool = True,
153154
**kwargs: Any,
154155
) -> None:
155156
self.logger = logging.getLogger(f"airbyte.{self.name}")
@@ -174,6 +175,7 @@ def __init__(
174175
# the declarative models into runtime components. Concurrent sources will continue to checkpoint
175176
# incremental streams running in full refresh.
176177
component_factory = ModelToComponentFactory(
178+
custom_components_trusted=custom_components_trusted,
177179
emit_connector_builder_messages=emit_connector_builder_messages,
178180
message_repository=ConcurrentMessageRepository(queue, message_repository),
179181
configured_catalog=catalog,
@@ -197,6 +199,7 @@ def __init__(
197199
component_factory
198200
if component_factory
199201
else ModelToComponentFactory(
202+
custom_components_trusted=custom_components_trusted,
200203
emit_connector_builder_messages=emit_connector_builder_messages,
201204
max_concurrent_async_job_count=source_config.get("max_concurrent_async_job_count"),
202205
)

airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@
478478
ZipfileDecoder as ZipfileDecoderModel,
479479
)
480480
from airbyte_cdk.sources.declarative.parsers.custom_code_compiler import (
481+
INJECTED_MANIFEST,
481482
AirbyteCustomCodeNotPermittedError,
482483
custom_code_execution_permitted,
483484
)
@@ -704,8 +705,10 @@ def __init__(
704705
max_concurrent_async_job_count: Optional[int] = None,
705706
configured_catalog: Optional[ConfiguredAirbyteCatalog] = None,
706707
api_budget: Optional[APIBudget] = None,
708+
custom_components_trusted: bool = True,
707709
):
708710
self._init_mappings()
711+
self._custom_components_trusted = custom_components_trusted
709712
self._limit_pages_fetched_per_slice = limit_pages_fetched_per_slice
710713
self._limit_slices_fetched = limit_slices_fetched
711714
self._emit_connector_builder_messages = emit_connector_builder_messages
@@ -1814,11 +1817,15 @@ def create_custom_component(self, model: Any, config: Config, **kwargs: Any) ->
18141817
:return: The declarative component built from the Pydantic model to be used at runtime
18151818
"""
18161819
# Instantiating a custom component means importing and executing arbitrary code referenced
1817-
# by `class_name`. This is only permitted when custom code execution is explicitly enabled,
1818-
# mirroring the gate applied to injected `components.py` code. Without this check, a manifest
1819-
# could point `class_name` at any importable callable and have it invoked, bypassing the
1820-
# `AIRBYTE_ENABLE_UNSAFE_CODE` protection.
1821-
if not custom_code_execution_permitted():
1820+
# by `class_name`. Manifests supplied by a caller, whether through the config or directly to
1821+
# the manifest server, are untrusted input and could point `class_name` at any importable
1822+
# callable, so they honor the same `AIRBYTE_ENABLE_UNSAFE_CODE` gate as injected
1823+
# `components.py` code. Manifests bundled in a published connector image are trusted and may
1824+
# always use their bundled custom components.
1825+
manifest_is_untrusted = not self._custom_components_trusted or bool(
1826+
config.get(INJECTED_MANIFEST)
1827+
)
1828+
if manifest_is_untrusted and not custom_code_execution_permitted():
18221829
raise AirbyteCustomCodeNotPermittedError
18231830

18241831
custom_component_class = self._get_class_from_fully_qualified_class_name(model.class_name)
@@ -4140,6 +4147,7 @@ def create_parent_stream_config_with_substream_wrapper(
41404147
)
41414148

41424149
substream_factory = ModelToComponentFactory(
4150+
custom_components_trusted=self._custom_components_trusted,
41434151
connector_state_manager=connector_state_manager,
41444152
limit_pages_fetched_per_slice=self._limit_pages_fetched_per_slice,
41454153
limit_slices_fetched=self._limit_slices_fetched,

unit_tests/connector_builder/test_connector_builder_handler.py

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@
5656
ConcurrentDeclarativeSource,
5757
TestLimits,
5858
)
59+
from airbyte_cdk.sources.declarative.parsers.custom_code_compiler import (
60+
ENV_VAR_ALLOW_CUSTOM_CODE,
61+
AirbyteCustomCodeNotPermittedError,
62+
)
5963
from airbyte_cdk.sources.declarative.retrievers.simple_retriever import SimpleRetriever
6064
from airbyte_cdk.sources.declarative.stream_slicers import StreamSlicerTestReadDecorator
6165
from airbyte_cdk.sources.streams.concurrent.default_stream import DefaultStream
@@ -1031,6 +1035,117 @@ def test_create_source():
10311035
assert source._constructor._disable_cache
10321036

10331037

1038+
def test_create_source_marks_manifest_untrusted():
1039+
"""A Connector Builder manifest is caller-supplied, so its factory must be untrusted.
1040+
1041+
This is the invariant that gates custom-component execution independently of the config
1042+
contents, so it cannot be defeated by a manifest that manipulates its own config.
1043+
"""
1044+
source = create_source(
1045+
config={"__injected_declarative_manifest": MANIFEST}, limits=None, catalog=None, state=None
1046+
)
1047+
1048+
assert source._constructor._custom_components_trusted is False
1049+
1050+
1051+
_GATE_BYPASS_STREAM = {
1052+
"type": "DeclarativeStream",
1053+
"name": "s",
1054+
"retriever": {
1055+
"type": "SimpleRetriever",
1056+
"requester": {
1057+
"type": "HttpRequester",
1058+
"url_base": "https://example.com",
1059+
"path": "/",
1060+
},
1061+
"record_selector": {
1062+
"type": "RecordSelector",
1063+
"extractor": {
1064+
"type": "CustomRecordExtractor",
1065+
"class_name": "not_a_real_module.NotAThing",
1066+
},
1067+
},
1068+
},
1069+
"schema_loader": {
1070+
"type": "InlineSchemaLoader",
1071+
"schema": {"type": "object", "properties": {}},
1072+
},
1073+
}
1074+
1075+
1076+
def test_spec_level_custom_component_is_gated(monkeypatch):
1077+
"""A `Custom*` component in `spec.config_normalization_rules` must honor the gate.
1078+
1079+
The spec component is built with an empty config, so the config-key provenance signal is
1080+
absent there; the untrusted-factory flag is what keeps the gate active. A non-existent
1081+
`class_name` proves the gate fires *before* the class is resolved (otherwise the failure
1082+
would be a class-resolution `ValueError`, not `AirbyteCustomCodeNotPermittedError`).
1083+
"""
1084+
monkeypatch.delenv(ENV_VAR_ALLOW_CUSTOM_CODE, raising=False)
1085+
manifest = {
1086+
"type": "DeclarativeSource",
1087+
"version": "6.0.0",
1088+
"check": {"type": "CheckStream", "stream_names": ["s"]},
1089+
"spec": {
1090+
"type": "Spec",
1091+
"connection_specification": {"type": "object", "properties": {}},
1092+
"config_normalization_rules": {
1093+
"type": "ConfigNormalizationRules",
1094+
"transformations": [
1095+
{
1096+
"type": "CustomConfigTransformation",
1097+
"class_name": "not_a_real_module.NotAThing",
1098+
}
1099+
],
1100+
},
1101+
},
1102+
"streams": [_GATE_BYPASS_STREAM],
1103+
}
1104+
1105+
with pytest.raises(AirbyteCustomCodeNotPermittedError):
1106+
create_source(
1107+
config={"__injected_declarative_manifest": manifest},
1108+
limits=None,
1109+
catalog=None,
1110+
state=None,
1111+
)
1112+
1113+
1114+
def test_config_strip_does_not_bypass_custom_code_gate(monkeypatch):
1115+
"""A manifest that strips its own provenance key must not escape the gate.
1116+
1117+
A plain `ConfigRemoveFields` removing `__injected_declarative_manifest` runs before streams
1118+
are built, defeating the config-key signal. The untrusted-factory flag does not live in the
1119+
config, so a stream-level `Custom*` component stays gated.
1120+
"""
1121+
monkeypatch.delenv(ENV_VAR_ALLOW_CUSTOM_CODE, raising=False)
1122+
manifest = {
1123+
"type": "DeclarativeSource",
1124+
"version": "6.0.0",
1125+
"check": {"type": "CheckStream", "stream_names": ["s"]},
1126+
"spec": {
1127+
"type": "Spec",
1128+
"connection_specification": {"type": "object", "properties": {}},
1129+
"config_normalization_rules": {
1130+
"type": "ConfigNormalizationRules",
1131+
"transformations": [
1132+
{
1133+
"type": "ConfigRemoveFields",
1134+
"field_pointers": [["__injected_declarative_manifest"]],
1135+
}
1136+
],
1137+
},
1138+
},
1139+
"streams": [_GATE_BYPASS_STREAM],
1140+
}
1141+
config = {"__injected_declarative_manifest": manifest}
1142+
1143+
source = create_source(config=config, limits=None, catalog=None, state=None)
1144+
1145+
with pytest.raises(AirbyteCustomCodeNotPermittedError):
1146+
source.streams(config)
1147+
1148+
10341149
def request_log_message(request: dict) -> AirbyteMessage:
10351150
return AirbyteMessage(
10361151
type=Type.LOG,

unit_tests/legacy/sources/declarative/conftest.py

Lines changed: 0 additions & 33 deletions
This file was deleted.

unit_tests/source_declarative_manifest/test_source_declarative_w_custom_components.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,34 @@ def _read_fn(*args, **kwargs):
288288
_read_fn()
289289

290290

291-
def test_register_components_from_file(components_file: str) -> None:
292-
"""Test that components can be properly loaded from a file."""
291+
def test_register_components_from_file(
292+
components_file: str,
293+
monkeypatch: pytest.MonkeyPatch,
294+
) -> None:
295+
"""Test that components can be properly loaded from a file when custom code is permitted."""
296+
monkeypatch.setenv(ENV_VAR_ALLOW_CUSTOM_CODE, "true")
297+
293298
# Register the components
294299
_register_components_from_file(components_file)
295300

296301
# Verify the components were loaded correctly
297302
verify_components_loaded()
303+
304+
305+
def test_register_components_from_file_is_gated(
306+
components_file: str,
307+
monkeypatch: pytest.MonkeyPatch,
308+
) -> None:
309+
"""A `--components-path` file is caller-supplied Python executed via `exec_module`.
310+
311+
It must be gated behind `AIRBYTE_ENABLE_UNSAFE_CODE`: with the env var unset the file must
312+
not be executed and the loader must raise `AirbyteCustomCodeNotPermittedError` before any
313+
module-level code runs.
314+
"""
315+
monkeypatch.delenv(ENV_VAR_ALLOW_CUSTOM_CODE, raising=False)
316+
317+
with pytest.raises(AirbyteCustomCodeNotPermittedError):
318+
_register_components_from_file(components_file)
319+
320+
assert "components" not in sys.modules
321+
assert "source_declarative_manifest.components" not in sys.modules

unit_tests/sources/declarative/conftest.py

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)