Skip to content

Commit

Permalink
Merge pull request from GHSA-p6p2-qq95-vq5h
Browse files Browse the repository at this point in the history
* Removing custom connector template functions

* Updating connector template modal description

* Adding function upload tests
  • Loading branch information
galvana authored Sep 5, 2023
1 parent 993bf88 commit 5989b5f
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 414 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ const ConnectorTemplateUploadModal: React.FC<RequestModalProps> = ({
</Box>
<Text fontSize="sm" mt={4}>
An integration template zip file must include a SaaS config and
dataset, but may also contain an icon (.svg) and custom functions
(.py) as optional files.
dataset, but may also contain an icon (.svg) as an optional file.
</Text>
</ModalBody>
<ModalFooter>
Expand Down
1 change: 0 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ services:
FIDES__DEV_MODE: "True"
FIDES__LOGGING__COLORIZE: "True"
FIDES__USER__ANALYTICS_OPT_OUT: "True"
FIDES__SECURITY__ALLOW_CUSTOM_CONNECTOR_FUNCTIONS: "True"
FIDES__SECURITY__BASTION_SERVER_HOST: ${FIDES__SECURITY__BASTION_SERVER_HOST-}
FIDES__SECURITY__BASTION_SERVER_SSH_USERNAME: ${FIDES__SECURITY__BASTION_SERVER_SSH_USERNAME-}
FIDES__SECURITY__BASTION_SERVER_SSH_PRIVATE_KEY: ${FIDES__SECURITY__BASTION_SERVER_SSH_PRIVATE_KEY-}
Expand Down
2 changes: 0 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
AccessControl==6.0
alembic==1.8.1
APScheduler==3.9.1.post1
asyncpg==0.27.0
Expand Down Expand Up @@ -42,7 +41,6 @@ pymssql==2.2.8
python-jose[cryptography]==3.3.0
pyyaml==6.0.1
redis==3.5.3
RestrictedPython==6.0.0
rich-click==1.6.1
sendgrid==6.9.7
slowapi==0.1.8
Expand Down
3 changes: 1 addition & 2 deletions src/fides/api/schemas/saas/connector_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@
class ConnectorTemplate(BaseModel):
"""
A collection of artifacts that make up a complete
SaaS connector (SaaS config, dataset, icon, functions, etc.)
SaaS connector (SaaS config, dataset, icon, etc.)
"""

config: str
dataset: str
icon: Optional[str]
functions: Optional[str]
human_readable: str

@validator("config")
Expand Down
108 changes: 4 additions & 104 deletions src/fides/api/service/connectors/saas/connector_registry_service.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,17 @@
# pylint: disable=protected-access
import os
from abc import ABC, abstractmethod
from ast import AST, AnnAssign
from operator import getitem
from typing import Any, Dict, Iterable, List, Optional, Tuple, Type
from typing import Dict, Iterable, List, Optional, Type
from zipfile import ZipFile

from AccessControl.ZopeGuards import safe_builtins
from fideslang.models import Dataset
from loguru import logger
from packaging.version import Version
from packaging.version import parse as parse_version
from RestrictedPython import compile_restricted
from RestrictedPython.transformer import RestrictingNodeTransformer
from sqlalchemy.orm import Session

from fides.api.api.deps import get_api_session
from fides.api.common_exceptions import FidesopsException, ValidationError
from fides.api.common_exceptions import ValidationError
from fides.api.cryptography.cryptographic_util import str_to_b64_str
from fides.api.models.connectionconfig import (
AccessLevel,
Expand All @@ -41,7 +36,6 @@
replace_version,
)
from fides.api.util.unsafe_file_util import verify_svg, verify_zip
from fides.config import CONFIG


class ConnectorTemplateLoader(ABC):
Expand Down Expand Up @@ -96,7 +90,6 @@ def _load_connector_templates(self) -> None:
f"data/saas/dataset/{connector_type}_dataset.yml"
),
icon=icon,
functions=None,
human_readable=human_readable,
)
except Exception:
Expand Down Expand Up @@ -151,24 +144,16 @@ def _register_template(
template: CustomConnectorTemplate,
) -> None:
"""
Registers a custom connector template by converting it to a ConnectorTemplate,
registering any custom functions, and adding it to the loader's template dictionary.
Registers a custom connector template by converting it to a ConnectorTemplate
and adding it to the loader's template dictionary.
"""
connector_template = ConnectorTemplate(
config=template.config,
dataset=template.dataset,
icon=template.icon,
functions=template.functions,
human_readable=template.name,
)

# register custom functions if available
if template.functions:
register_custom_functions(template.functions)
logger.info(
f"Loaded functions from the custom connector template '{template.key}'"
)

# register the template in the loader's template dictionary
CustomConnectorTemplateLoader.get_connector_templates()[
template.key
Expand Down Expand Up @@ -220,13 +205,6 @@ def save_template(cls, db: Session, zip_file: ZipFile) -> None:
raise ValidationError(
"Multiple svg files found, only one is allowed."
)
elif info.filename.endswith(".py"):
if not function_contents:
function_contents = file_contents
else:
raise ValidationError(
"Multiple Python (.py) files found, only one is allowed."
)

if not config_contents:
raise ValidationError("Zip file does not contain a config.yml file.")
Expand Down Expand Up @@ -266,7 +244,6 @@ def save_template(cls, db: Session, zip_file: ZipFile) -> None:
config=config_contents,
dataset=dataset_contents,
icon=icon_contents,
functions=function_contents,
replaceable=replaceable,
)

Expand Down Expand Up @@ -447,80 +424,3 @@ def update_saas_instance(
connection_config.update_saas_config(db, SaaSConfig(**config_from_template))

upsert_dataset_config_from_template(db, connection_config, template, template_vals)


def register_custom_functions(script: str) -> None:
"""
Registers custom functions by executing the given script in a restricted environment.
The script is compiled and executed with RestrictedPython, which is designed to reduce
the risk of executing untrusted code. It provides a set of safe builtins to prevent
malicious or unintended behavior.
Args:
script (str): The Python script containing the custom functions to be registered.
Raises:
FidesopsException: If allow_custom_connector_functions is disabled.
SyntaxError: If the script contains a syntax error or uses restricted language features.
Exception: If an exception occurs during the execution of the script.
"""

if CONFIG.security.allow_custom_connector_functions:
restricted_code = compile_restricted(
script, "<string>", "exec", policy=CustomRestrictingNodeTransformer
)
safe_builtins["__import__"] = custom_guarded_import
safe_builtins["_getitem_"] = getitem
safe_builtins["staticmethod"] = staticmethod

# pylint: disable=exec-used
exec(
restricted_code,
{
"__metaclass__": type,
"__name__": "restricted_module",
"__builtins__": safe_builtins,
},
)
else:
raise FidesopsException(
message="The import of connector templates with custom functions is disabled by the 'security.allow_custom_connector_functions' setting."
)


class CustomRestrictingNodeTransformer(RestrictingNodeTransformer):
"""
Custom node transformer class that extends RestrictedPython's RestrictingNodeTransformer
to allow the use of type annotations (AnnAssign) in restricted code.
"""

def visit_AnnAssign(self, node: AnnAssign) -> AST:
return self.node_contents_visit(node)


def custom_guarded_import(
name: str,
_globals: Optional[dict] = None,
_locals: Optional[dict] = None,
fromlist: Optional[Tuple[str, ...]] = None,
level: int = 0,
) -> Any:
"""
A custom import function that prevents the import of certain potentially unsafe modules.
"""
if name in [
"os",
"sys",
"subprocess",
"shutil",
"socket",
"importlib",
"tempfile",
"glob",
]:
# raising SyntaxError to be consistent with exceptions thrown from other guarded functions
raise SyntaxError(f"Import of '{name}' module is not allowed.")
if fromlist is None:
fromlist = ()
return __import__(name, _globals, _locals, fromlist, level)
4 changes: 0 additions & 4 deletions src/fides/config/security_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,6 @@ class SecuritySettings(FidesSettings):
default=432000,
description="The number of seconds that a pre-signed download URL when using S3 storage will be valid. The default is equal to 5 days.",
)
allow_custom_connector_functions: Optional[bool] = Field(
default=False,
description="Enables or disables the ability to import connector templates with custom functions. When enabled, custom functions which will be loaded in a restricted environment to minimize security risks.",
)
enable_audit_log_resource_middleware: Optional[bool] = Field(
default=False,
description="Either enables the collection of audit log resource data or bypasses the middleware",
Expand Down
Loading

0 comments on commit 5989b5f

Please sign in to comment.