diff --git a/autogpt_platform/backend/backend/api/features/chat/tools/utils.py b/autogpt_platform/backend/backend/api/features/chat/tools/utils.py index bd25594b8a..029bb3f58d 100644 --- a/autogpt_platform/backend/backend/api/features/chat/tools/utils.py +++ b/autogpt_platform/backend/backend/api/features/chat/tools/utils.py @@ -124,7 +124,7 @@ def build_missing_credentials_from_graph( preserving all supported credential types for each field. """ matched_keys = set(matched_credentials.keys()) if matched_credentials else set() - aggregated_fields = graph.aggregate_credentials_inputs() + aggregated_fields = graph.regular_credentials_inputs return { field_key: _serialize_missing_credential(field_key, field_info) @@ -251,7 +251,7 @@ async def match_user_credentials_to_graph( missing_creds: list[str] = [] # Get aggregated credentials requirements from the graph - aggregated_creds = graph.aggregate_credentials_inputs() + aggregated_creds = graph.regular_credentials_inputs logger.debug( f"Matching credentials for graph {graph.id}: {len(aggregated_creds)} required" ) diff --git a/autogpt_platform/backend/backend/api/features/chat/tools/utils_test.py b/autogpt_platform/backend/backend/api/features/chat/tools/utils_test.py new file mode 100644 index 0000000000..b14dd130d9 --- /dev/null +++ b/autogpt_platform/backend/backend/api/features/chat/tools/utils_test.py @@ -0,0 +1,78 @@ +"""Tests for chat tools utility functions.""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from backend.data.model import CredentialsFieldInfo + + +def _make_regular_field() -> CredentialsFieldInfo: + return CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["github"], + "credentials_types": ["api_key"], + "is_auto_credential": False, + }, + by_alias=True, + ) + + +def test_build_missing_credentials_excludes_auto_creds(): + """ + build_missing_credentials_from_graph() should use regular_credentials_inputs + and thus exclude auto_credentials from the "missing" set. + """ + from backend.api.features.chat.tools.utils import ( + build_missing_credentials_from_graph, + ) + + regular_field = _make_regular_field() + + mock_graph = MagicMock() + # regular_credentials_inputs should only return the non-auto field + mock_graph.regular_credentials_inputs = { + "github_api_key": (regular_field, {("node-1", "credentials")}), + } + + result = build_missing_credentials_from_graph(mock_graph, matched_credentials=None) + + # Should include the regular credential + assert "github_api_key" in result + # Should NOT include the auto_credential (not in regular_credentials_inputs) + assert "google_oauth2" not in result + + +@pytest.mark.asyncio +async def test_match_user_credentials_excludes_auto_creds(): + """ + match_user_credentials_to_graph() should use regular_credentials_inputs + and thus exclude auto_credentials from matching. + """ + from backend.api.features.chat.tools.utils import match_user_credentials_to_graph + + regular_field = _make_regular_field() + + mock_graph = MagicMock() + mock_graph.id = "test-graph" + # regular_credentials_inputs returns only non-auto fields + mock_graph.regular_credentials_inputs = { + "github_api_key": (regular_field, {("node-1", "credentials")}), + } + + # Mock the credentials manager to return no credentials + with patch( + "backend.api.features.chat.tools.utils.IntegrationCredentialsManager" + ) as MockCredsMgr: + mock_store = AsyncMock() + mock_store.get_all_creds.return_value = [] + MockCredsMgr.return_value.store = mock_store + + matched, missing = await match_user_credentials_to_graph( + user_id="test-user", graph=mock_graph + ) + + # No credentials available, so github should be missing + assert len(matched) == 0 + assert len(missing) == 1 + assert "github_api_key" in missing[0] diff --git a/autogpt_platform/backend/backend/api/features/library/db.py b/autogpt_platform/backend/backend/api/features/library/db.py index 6bebfb573c..ee04682024 100644 --- a/autogpt_platform/backend/backend/api/features/library/db.py +++ b/autogpt_platform/backend/backend/api/features/library/db.py @@ -1103,7 +1103,7 @@ async def create_preset_from_graph_execution( raise NotFoundError( f"Graph #{graph_execution.graph_id} not found or accessible" ) - elif len(graph.aggregate_credentials_inputs()) > 0: + elif len(graph.regular_credentials_inputs) > 0: raise ValueError( f"Graph execution #{graph_exec_id} can't be turned into a preset " "because it was run before this feature existed " diff --git a/autogpt_platform/backend/backend/data/block.py b/autogpt_platform/backend/backend/data/block.py index eb9360b037..ed07c664e1 100644 --- a/autogpt_platform/backend/backend/data/block.py +++ b/autogpt_platform/backend/backend/data/block.py @@ -317,6 +317,8 @@ class BlockSchema(BaseModel): "credentials_provider": [config.get("provider", "google")], "credentials_types": [config.get("type", "oauth2")], "credentials_scopes": config.get("scopes"), + "is_auto_credential": True, + "input_field_name": info["field_name"], } result[kwarg_name] = CredentialsFieldInfo.model_validate( auto_schema, by_alias=True diff --git a/autogpt_platform/backend/backend/data/graph.py b/autogpt_platform/backend/backend/data/graph.py index ee6cd2e4b0..0bcec5aede 100644 --- a/autogpt_platform/backend/backend/data/graph.py +++ b/autogpt_platform/backend/backend/data/graph.py @@ -369,7 +369,7 @@ class Graph(BaseGraph): schema = self._credentials_input_schema.jsonschema() # Determine which credential fields are required based on credentials_optional metadata - graph_credentials_inputs = self.aggregate_credentials_inputs() + graph_credentials_inputs = self.regular_credentials_inputs required_fields = [] # Build a map of node_id -> node for quick lookup @@ -398,7 +398,7 @@ class Graph(BaseGraph): @property def _credentials_input_schema(self) -> type[BlockSchema]: - graph_credentials_inputs = self.aggregate_credentials_inputs() + graph_credentials_inputs = self.regular_credentials_inputs logger.debug( f"Combined credentials input fields for graph #{self.id} ({self.name}): " f"{graph_credentials_inputs}" @@ -487,6 +487,28 @@ class Graph(BaseGraph): # Combine credential field info (this will merge discriminator_values automatically) return CredentialsFieldInfo.combine(*node_credential_data) + @property + def regular_credentials_inputs( + self, + ) -> dict[str, tuple[CredentialsFieldInfo, set[tuple[str, str]]]]: + """Credentials that need explicit user mapping (CredentialsMetaInput fields).""" + return { + k: v + for k, v in self.aggregate_credentials_inputs().items() + if not v[0].is_auto_credential + } + + @property + def auto_credentials_inputs( + self, + ) -> dict[str, tuple[CredentialsFieldInfo, set[tuple[str, str]]]]: + """Credentials embedded in file fields (_credentials_id), resolved at execution time.""" + return { + k: v + for k, v in self.aggregate_credentials_inputs().items() + if v[0].is_auto_credential + } + class GraphModel(Graph): user_id: str @@ -567,6 +589,16 @@ class GraphModel(Graph): ) and graph_id in graph_id_map: node.input_default["graph_id"] = graph_id_map[graph_id] + # Clear auto-credentials references (e.g., _credentials_id in + # GoogleDriveFile fields) so the new user must re-authenticate + # with their own account + for node in graph.nodes: + if not node.input_default: + continue + for key, value in node.input_default.items(): + if isinstance(value, dict) and "_credentials_id" in value: + value["_credentials_id"] = None + def validate_graph( self, for_run: bool = False, diff --git a/autogpt_platform/backend/backend/data/graph_test.py b/autogpt_platform/backend/backend/data/graph_test.py index 8b7eadb887..d9a1adbbc0 100644 --- a/autogpt_platform/backend/backend/data/graph_test.py +++ b/autogpt_platform/backend/backend/data/graph_test.py @@ -463,3 +463,313 @@ def test_node_credentials_optional_with_other_metadata(): assert node.credentials_optional is True assert node.metadata["position"] == {"x": 100, "y": 200} assert node.metadata["customized_name"] == "My Custom Node" + + +# ============================================================================ +# Tests for _reassign_ids credential clearing (Fix 3: SECRT-1772) +def test_combine_preserves_is_auto_credential_flag(): + """ + CredentialsFieldInfo.combine() must propagate is_auto_credential and + input_field_name to the combined result. Regression test for reviewer + finding that combine() dropped these fields. + """ + from backend.data.model import CredentialsFieldInfo + + auto_field = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["google"], + "credentials_types": ["oauth2"], + "credentials_scopes": ["drive.readonly"], + "is_auto_credential": True, + "input_field_name": "spreadsheet", + }, + by_alias=True, + ) + + # combine() takes *args of (field_info, key) tuples + combined = CredentialsFieldInfo.combine( + (auto_field, ("node-1", "credentials")), + (auto_field, ("node-2", "credentials")), + ) + + assert len(combined) == 1 + group_key = next(iter(combined)) + combined_info, combined_keys = combined[group_key] + + assert combined_info.is_auto_credential is True + assert combined_info.input_field_name == "spreadsheet" + assert combined_keys == {("node-1", "credentials"), ("node-2", "credentials")} + + +def test_combine_preserves_regular_credential_defaults(): + """Regular credentials should have is_auto_credential=False after combine().""" + from backend.data.model import CredentialsFieldInfo + + regular_field = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["github"], + "credentials_types": ["api_key"], + "is_auto_credential": False, + }, + by_alias=True, + ) + + combined = CredentialsFieldInfo.combine( + (regular_field, ("node-1", "credentials")), + ) + + group_key = next(iter(combined)) + combined_info, _ = combined[group_key] + + assert combined_info.is_auto_credential is False + assert combined_info.input_field_name is None + + +# ============================================================================ + + +def test_reassign_ids_clears_credentials_id(): + """ + [SECRT-1772] _reassign_ids should clear _credentials_id from + GoogleDriveFile-style input_default fields so forked agents + don't retain the original creator's credential references. + """ + from backend.data.graph import GraphModel + + node = Node( + id="node-1", + block_id=StoreValueBlock().id, + input_default={ + "spreadsheet": { + "_credentials_id": "original-cred-id", + "id": "file-123", + "name": "test.xlsx", + "mimeType": "application/vnd.google-apps.spreadsheet", + "url": "https://docs.google.com/spreadsheets/d/file-123", + }, + }, + ) + + graph = Graph( + id="test-graph", + name="Test", + description="Test", + nodes=[node], + links=[], + ) + + GraphModel._reassign_ids(graph, user_id="new-user", graph_id_map={}) + + # _credentials_id should be cleared + assert graph.nodes[0].input_default["spreadsheet"]["_credentials_id"] is None + + +def test_reassign_ids_preserves_non_credential_fields(): + """ + Regression guard: _reassign_ids should NOT modify non-credential fields + like name, mimeType, id, url. + """ + from backend.data.graph import GraphModel + + node = Node( + id="node-1", + block_id=StoreValueBlock().id, + input_default={ + "spreadsheet": { + "_credentials_id": "cred-abc", + "id": "file-123", + "name": "test.xlsx", + "mimeType": "application/vnd.google-apps.spreadsheet", + "url": "https://docs.google.com/spreadsheets/d/file-123", + }, + }, + ) + + graph = Graph( + id="test-graph", + name="Test", + description="Test", + nodes=[node], + links=[], + ) + + GraphModel._reassign_ids(graph, user_id="new-user", graph_id_map={}) + + field = graph.nodes[0].input_default["spreadsheet"] + assert field["id"] == "file-123" + assert field["name"] == "test.xlsx" + assert field["mimeType"] == "application/vnd.google-apps.spreadsheet" + assert field["url"] == "https://docs.google.com/spreadsheets/d/file-123" + + +def test_reassign_ids_handles_no_credentials(): + """ + Regression guard: _reassign_ids should not error when input_default + has no dict fields with _credentials_id. + """ + from backend.data.graph import GraphModel + + node = Node( + id="node-1", + block_id=StoreValueBlock().id, + input_default={ + "input": "some value", + "another_input": 42, + }, + ) + + graph = Graph( + id="test-graph", + name="Test", + description="Test", + nodes=[node], + links=[], + ) + + GraphModel._reassign_ids(graph, user_id="new-user", graph_id_map={}) + + # Should not error, fields unchanged + assert graph.nodes[0].input_default["input"] == "some value" + assert graph.nodes[0].input_default["another_input"] == 42 + + +def test_reassign_ids_handles_multiple_credential_fields(): + """ + [SECRT-1772] When a node has multiple dict fields with _credentials_id, + ALL of them should be cleared. + """ + from backend.data.graph import GraphModel + + node = Node( + id="node-1", + block_id=StoreValueBlock().id, + input_default={ + "spreadsheet": { + "_credentials_id": "cred-1", + "id": "file-1", + "name": "file1.xlsx", + }, + "doc_file": { + "_credentials_id": "cred-2", + "id": "file-2", + "name": "file2.docx", + }, + "plain_input": "not a dict", + }, + ) + + graph = Graph( + id="test-graph", + name="Test", + description="Test", + nodes=[node], + links=[], + ) + + GraphModel._reassign_ids(graph, user_id="new-user", graph_id_map={}) + + assert graph.nodes[0].input_default["spreadsheet"]["_credentials_id"] is None + assert graph.nodes[0].input_default["doc_file"]["_credentials_id"] is None + assert graph.nodes[0].input_default["plain_input"] == "not a dict" + + +# ============================================================================ +# Tests for discriminate() field propagation +def test_discriminate_preserves_is_auto_credential_flag(): + """ + CredentialsFieldInfo.discriminate() must propagate is_auto_credential and + input_field_name to the discriminated result. Regression test for + discriminate() dropping these fields (same class of bug as combine()). + """ + from backend.data.model import CredentialsFieldInfo + + auto_field = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["google", "openai"], + "credentials_types": ["oauth2"], + "credentials_scopes": ["drive.readonly"], + "is_auto_credential": True, + "input_field_name": "spreadsheet", + "discriminator": "model", + "discriminator_mapping": {"gpt-4": "openai", "gemini": "google"}, + }, + by_alias=True, + ) + + discriminated = auto_field.discriminate("gemini") + + assert discriminated.is_auto_credential is True + assert discriminated.input_field_name == "spreadsheet" + assert discriminated.provider == frozenset(["google"]) + + +def test_discriminate_preserves_regular_credential_defaults(): + """Regular credentials should have is_auto_credential=False after discriminate().""" + from backend.data.model import CredentialsFieldInfo + + regular_field = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["google", "openai"], + "credentials_types": ["api_key"], + "is_auto_credential": False, + "discriminator": "model", + "discriminator_mapping": {"gpt-4": "openai", "gemini": "google"}, + }, + by_alias=True, + ) + + discriminated = regular_field.discriminate("gpt-4") + + assert discriminated.is_auto_credential is False + assert discriminated.input_field_name is None + assert discriminated.provider == frozenset(["openai"]) + + +# ============================================================================ +# Tests for credentials_input_schema excluding auto_credentials +def test_credentials_input_schema_excludes_auto_creds(): + """ + Graph._credentials_input_schema and credentials_input_schema should exclude + auto_credentials (is_auto_credential=True) from the schema. Auto_credentials + are transparently resolved at execution time via file picker data. + """ + from unittest.mock import PropertyMock, patch + + from backend.data.model import CredentialsFieldInfo + + regular_field_info = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["github"], + "credentials_types": ["api_key"], + "is_auto_credential": False, + }, + by_alias=True, + ) + + graph = Graph( + id="test-graph", + name="Test", + description="Test", + nodes=[ + Node(id="node-1", block_id=StoreValueBlock().id, input_default={}), + ], + links=[], + ) + + # Mock regular_credentials_inputs to return only the non-auto field + # Field names must match *_credentials pattern for BlockSchema validation + regular_only = { + "github_credentials": (regular_field_info, {("node-1", "credentials")}), + } + + with patch.object( + type(graph), + "regular_credentials_inputs", + new_callable=PropertyMock, + return_value=regular_only, + ): + schema = graph._credentials_input_schema + field_names = set(schema.model_fields.keys()) + # Should include regular credential but NOT auto_credential + assert "github_credentials" in field_names + assert "google_credentials" not in field_names diff --git a/autogpt_platform/backend/backend/data/model.py b/autogpt_platform/backend/backend/data/model.py index 5a09c591c9..c3cac02df1 100644 --- a/autogpt_platform/backend/backend/data/model.py +++ b/autogpt_platform/backend/backend/data/model.py @@ -574,6 +574,8 @@ class CredentialsFieldInfo(BaseModel, Generic[CP, CT]): discriminator: Optional[str] = None discriminator_mapping: Optional[dict[str, CP]] = None discriminator_values: set[Any] = Field(default_factory=set) + is_auto_credential: bool = False + input_field_name: Optional[str] = None @classmethod def combine( @@ -654,6 +656,9 @@ class CredentialsFieldInfo(BaseModel, Generic[CP, CT]): + "_credentials" ) + # Propagate is_auto_credential from the combined field. + # All fields in a group should share the same is_auto_credential + # value since auto and regular credentials serve different purposes. result[group_key] = ( CredentialsFieldInfo[CP, CT]( credentials_provider=combined.provider, @@ -662,6 +667,8 @@ class CredentialsFieldInfo(BaseModel, Generic[CP, CT]): discriminator=combined.discriminator, discriminator_mapping=combined.discriminator_mapping, discriminator_values=set(all_discriminator_values), + is_auto_credential=combined.is_auto_credential, + input_field_name=combined.input_field_name, ), combined_keys, ) @@ -687,6 +694,8 @@ class CredentialsFieldInfo(BaseModel, Generic[CP, CT]): discriminator=self.discriminator, discriminator_mapping=self.discriminator_mapping, discriminator_values=self.discriminator_values, + is_auto_credential=self.is_auto_credential, + input_field_name=self.input_field_name, ) diff --git a/autogpt_platform/backend/backend/executor/manager.py b/autogpt_platform/backend/backend/executor/manager.py index 8362dae828..40b386a359 100644 --- a/autogpt_platform/backend/backend/executor/manager.py +++ b/autogpt_platform/backend/backend/executor/manager.py @@ -172,6 +172,81 @@ def execute_graph( T = TypeVar("T") +async def _acquire_auto_credentials( + input_model: type[BlockSchema], + input_data: dict[str, Any], + creds_manager: "IntegrationCredentialsManager", + user_id: str, +) -> tuple[dict[str, Any], list[AsyncRedisLock]]: + """ + Resolve auto_credentials from GoogleDriveFileField-style inputs. + + Returns: + (extra_exec_kwargs, locks): kwargs to inject into block execution, and + credential locks to release after execution completes. + """ + extra_exec_kwargs: dict[str, Any] = {} + locks: list[AsyncRedisLock] = [] + + # NOTE: If a block ever has multiple auto-credential fields, a ValueError + # on a later field will strand locks acquired for earlier fields. They'll + # auto-expire via Redis TTL, but add a try/except to release partial locks + # if that becomes a real scenario. + for kwarg_name, info in input_model.get_auto_credentials_fields().items(): + field_name = info["field_name"] + field_data = input_data.get(field_name) + + if field_data and isinstance(field_data, dict): + # Check if _credentials_id key exists in the field data + if "_credentials_id" in field_data: + cred_id = field_data["_credentials_id"] + if cred_id: + # Credential ID provided - acquire credentials + provider = info.get("config", {}).get( + "provider", "external service" + ) + file_name = field_data.get("name", "selected file") + try: + credentials, lock = await creds_manager.acquire( + user_id, cred_id + ) + locks.append(lock) + extra_exec_kwargs[kwarg_name] = credentials + except ValueError: + raise ValueError( + f"{provider.capitalize()} credentials for " + f"'{file_name}' in field '{field_name}' are not " + f"available in your account. " + f"This can happen if the agent was created by another " + f"user or the credentials were deleted. " + f"Please open the agent in the builder and re-select " + f"the file to authenticate with your own account." + ) + # else: _credentials_id is explicitly None, skip (chained data) + else: + # _credentials_id key missing entirely - this is an error + provider = info.get("config", {}).get("provider", "external service") + file_name = field_data.get("name", "selected file") + raise ValueError( + f"Authentication missing for '{file_name}' in field " + f"'{field_name}'. Please re-select the file to authenticate " + f"with {provider.capitalize()}." + ) + elif field_data is None and field_name not in input_data: + # Field not in input_data at all = connected from upstream block, skip + pass + else: + # field_data is None/empty but key IS in input_data = user didn't select + provider = info.get("config", {}).get("provider", "external service") + raise ValueError( + f"No file selected for '{field_name}'. " + f"Please select a file to provide " + f"{provider.capitalize()} authentication." + ) + + return extra_exec_kwargs, locks + + async def execute_node( node: Node, data: NodeExecutionEntry, @@ -271,41 +346,14 @@ async def execute_node( extra_exec_kwargs[field_name] = credentials # Handle auto-generated credentials (e.g., from GoogleDriveFileInput) - for kwarg_name, info in input_model.get_auto_credentials_fields().items(): - field_name = info["field_name"] - field_data = input_data.get(field_name) - if field_data and isinstance(field_data, dict): - # Check if _credentials_id key exists in the field data - if "_credentials_id" in field_data: - cred_id = field_data["_credentials_id"] - if cred_id: - # Credential ID provided - acquire credentials - provider = info.get("config", {}).get( - "provider", "external service" - ) - file_name = field_data.get("name", "selected file") - try: - credentials, lock = await creds_manager.acquire( - user_id, cred_id - ) - creds_locks.append(lock) - extra_exec_kwargs[kwarg_name] = credentials - except ValueError: - # Credential was deleted or doesn't exist - raise ValueError( - f"Authentication expired for '{file_name}' in field '{field_name}'. " - f"The saved {provider.capitalize()} credentials no longer exist. " - f"Please re-select the file to re-authenticate." - ) - # else: _credentials_id is explicitly None, skip credentials (for chained data) - else: - # _credentials_id key missing entirely - this is an error - provider = info.get("config", {}).get("provider", "external service") - file_name = field_data.get("name", "selected file") - raise ValueError( - f"Authentication missing for '{file_name}' in field '{field_name}'. " - f"Please re-select the file to authenticate with {provider.capitalize()}." - ) + auto_extra_kwargs, auto_locks = await _acquire_auto_credentials( + input_model=input_model, + input_data=input_data, + creds_manager=creds_manager, + user_id=user_id, + ) + extra_exec_kwargs.update(auto_extra_kwargs) + creds_locks.extend(auto_locks) output_size = 0 diff --git a/autogpt_platform/backend/backend/executor/manager_auto_credentials_test.py b/autogpt_platform/backend/backend/executor/manager_auto_credentials_test.py new file mode 100644 index 0000000000..6b337c6706 --- /dev/null +++ b/autogpt_platform/backend/backend/executor/manager_auto_credentials_test.py @@ -0,0 +1,320 @@ +""" +Tests for auto_credentials handling in execute_node(). + +These test the _acquire_auto_credentials() helper function extracted from +execute_node() (manager.py lines 273-308). +""" + +import pytest +from pytest_mock import MockerFixture + + +@pytest.fixture +def google_drive_file_data(): + return { + "valid": { + "_credentials_id": "cred-id-123", + "id": "file-123", + "name": "test.xlsx", + "mimeType": "application/vnd.google-apps.spreadsheet", + }, + "chained": { + "_credentials_id": None, + "id": "file-456", + "name": "chained.xlsx", + "mimeType": "application/vnd.google-apps.spreadsheet", + }, + "missing_key": { + "id": "file-789", + "name": "bad.xlsx", + "mimeType": "application/vnd.google-apps.spreadsheet", + }, + } + + +@pytest.fixture +def mock_input_model(mocker: MockerFixture): + """Create a mock input model with get_auto_credentials_fields() returning one field.""" + input_model = mocker.MagicMock() + input_model.get_auto_credentials_fields.return_value = { + "credentials": { + "field_name": "spreadsheet", + "config": { + "provider": "google", + "type": "oauth2", + "scopes": ["https://www.googleapis.com/auth/drive.readonly"], + }, + } + } + return input_model + + +@pytest.fixture +def mock_creds_manager(mocker: MockerFixture): + manager = mocker.AsyncMock() + mock_lock = mocker.AsyncMock() + mock_creds = mocker.MagicMock() + mock_creds.id = "cred-id-123" + mock_creds.provider = "google" + manager.acquire.return_value = (mock_creds, mock_lock) + return manager, mock_creds, mock_lock + + +@pytest.mark.asyncio +async def test_auto_credentials_happy_path( + mocker: MockerFixture, + google_drive_file_data, + mock_input_model, + mock_creds_manager, +): + """When field_data has a valid _credentials_id, credentials should be acquired.""" + from backend.executor.manager import _acquire_auto_credentials + + manager, mock_creds, mock_lock = mock_creds_manager + input_data = {"spreadsheet": google_drive_file_data["valid"]} + + extra_kwargs, locks = await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + manager.acquire.assert_called_once_with("user-1", "cred-id-123") + assert extra_kwargs["credentials"] == mock_creds + assert mock_lock in locks + + +@pytest.mark.asyncio +async def test_auto_credentials_field_none_static_raises( + mocker: MockerFixture, + mock_input_model, + mock_creds_manager, +): + """ + [THE BUG FIX TEST — OPEN-2895] + When field_data is None and the key IS in input_data (user didn't select a file), + should raise ValueError instead of silently skipping. + """ + from backend.executor.manager import _acquire_auto_credentials + + manager, _, _ = mock_creds_manager + # Key is present but value is None = user didn't select a file + input_data = {"spreadsheet": None} + + with pytest.raises(ValueError, match="No file selected"): + await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + +@pytest.mark.asyncio +async def test_auto_credentials_field_absent_skips( + mocker: MockerFixture, + mock_input_model, + mock_creds_manager, +): + """ + When the field key is NOT in input_data at all (upstream connection), + should skip without error. + """ + from backend.executor.manager import _acquire_auto_credentials + + manager, _, _ = mock_creds_manager + # Key not present = connected from upstream block + input_data = {} + + extra_kwargs, locks = await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + manager.acquire.assert_not_called() + assert "credentials" not in extra_kwargs + assert locks == [] + + +@pytest.mark.asyncio +async def test_auto_credentials_chained_cred_id_none( + mocker: MockerFixture, + google_drive_file_data, + mock_input_model, + mock_creds_manager, +): + """ + When _credentials_id is explicitly None (chained data from upstream), + should skip credential acquisition. + """ + from backend.executor.manager import _acquire_auto_credentials + + manager, _, _ = mock_creds_manager + input_data = {"spreadsheet": google_drive_file_data["chained"]} + + extra_kwargs, locks = await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + manager.acquire.assert_not_called() + assert "credentials" not in extra_kwargs + + +@pytest.mark.asyncio +async def test_auto_credentials_missing_cred_id_key_raises( + mocker: MockerFixture, + google_drive_file_data, + mock_input_model, + mock_creds_manager, +): + """ + When _credentials_id key is missing entirely from field_data dict, + should raise ValueError. + """ + from backend.executor.manager import _acquire_auto_credentials + + manager, _, _ = mock_creds_manager + input_data = {"spreadsheet": google_drive_file_data["missing_key"]} + + with pytest.raises(ValueError, match="Authentication missing"): + await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + +@pytest.mark.asyncio +async def test_auto_credentials_ownership_mismatch_error( + mocker: MockerFixture, + google_drive_file_data, + mock_input_model, + mock_creds_manager, +): + """ + [SECRT-1772] When acquire() raises ValueError (credential belongs to another user), + the error message should mention 'not available' (not 'expired'). + """ + from backend.executor.manager import _acquire_auto_credentials + + manager, _, _ = mock_creds_manager + manager.acquire.side_effect = ValueError( + "Credentials #cred-id-123 for user #user-2 not found" + ) + input_data = {"spreadsheet": google_drive_file_data["valid"]} + + with pytest.raises(ValueError, match="not available in your account"): + await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-2", + ) + + +@pytest.mark.asyncio +async def test_auto_credentials_deleted_credential_error( + mocker: MockerFixture, + google_drive_file_data, + mock_input_model, + mock_creds_manager, +): + """ + [SECRT-1772] When acquire() raises ValueError (credential was deleted), + the error message should mention 'not available' (not 'expired'). + """ + from backend.executor.manager import _acquire_auto_credentials + + manager, _, _ = mock_creds_manager + manager.acquire.side_effect = ValueError( + "Credentials #cred-id-123 for user #user-1 not found" + ) + input_data = {"spreadsheet": google_drive_file_data["valid"]} + + with pytest.raises(ValueError, match="not available in your account"): + await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + +@pytest.mark.asyncio +async def test_auto_credentials_lock_appended( + mocker: MockerFixture, + google_drive_file_data, + mock_input_model, + mock_creds_manager, +): + """Lock from acquire() should be included in returned locks list.""" + from backend.executor.manager import _acquire_auto_credentials + + manager, _, mock_lock = mock_creds_manager + input_data = {"spreadsheet": google_drive_file_data["valid"]} + + extra_kwargs, locks = await _acquire_auto_credentials( + input_model=mock_input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + assert len(locks) == 1 + assert locks[0] is mock_lock + + +@pytest.mark.asyncio +async def test_auto_credentials_multiple_fields( + mocker: MockerFixture, + mock_creds_manager, +): + """When there are multiple auto_credentials fields, only valid ones should acquire.""" + from backend.executor.manager import _acquire_auto_credentials + + manager, mock_creds, mock_lock = mock_creds_manager + + input_model = mocker.MagicMock() + input_model.get_auto_credentials_fields.return_value = { + "credentials": { + "field_name": "spreadsheet", + "config": {"provider": "google", "type": "oauth2"}, + }, + "credentials2": { + "field_name": "doc_file", + "config": {"provider": "google", "type": "oauth2"}, + }, + } + + input_data = { + "spreadsheet": { + "_credentials_id": "cred-id-123", + "id": "file-1", + "name": "file1.xlsx", + }, + "doc_file": { + "_credentials_id": None, + "id": "file-2", + "name": "chained.doc", + }, + } + + extra_kwargs, locks = await _acquire_auto_credentials( + input_model=input_model, + input_data=input_data, + creds_manager=manager, + user_id="user-1", + ) + + # Only the first field should have acquired credentials + manager.acquire.assert_called_once_with("user-1", "cred-id-123") + assert "credentials" in extra_kwargs + assert "credentials2" not in extra_kwargs + assert len(locks) == 1 diff --git a/autogpt_platform/backend/backend/executor/utils.py b/autogpt_platform/backend/backend/executor/utils.py index fa264c30a7..b324983a3e 100644 --- a/autogpt_platform/backend/backend/executor/utils.py +++ b/autogpt_platform/backend/backend/executor/utils.py @@ -259,7 +259,8 @@ async def _validate_node_input_credentials( # Find any fields of type CredentialsMetaInput credentials_fields = block.input_schema.get_credentials_fields() - if not credentials_fields: + auto_credentials_fields = block.input_schema.get_auto_credentials_fields() + if not credentials_fields and not auto_credentials_fields: continue # Track if any credential field is missing for this node @@ -339,6 +340,31 @@ async def _validate_node_input_credentials( ] = "Invalid credentials: type/provider mismatch" continue + # Validate auto-credentials (GoogleDriveFileField-based) + # These have _credentials_id embedded in the file field data + if auto_credentials_fields: + for _kwarg_name, info in auto_credentials_fields.items(): + field_name = info["field_name"] + # Check input_default and nodes_input_masks for the field value + field_value = node.input_default.get(field_name) + if nodes_input_masks and node.id in nodes_input_masks: + field_value = nodes_input_masks[node.id].get( + field_name, field_value + ) + + if field_value and isinstance(field_value, dict): + cred_id = field_value.get("_credentials_id") + if cred_id and isinstance(cred_id, str): + creds_store = get_integration_credentials_store() + creds = await creds_store.get_creds_by_id(user_id, cred_id) + if not creds: + has_missing_credentials = True + credential_errors[node.id][field_name] = ( + "The saved Google credentials are not available " + "for your account. Please re-select the file to " + "authenticate with your own Google account." + ) + # If node has optional credentials and any are missing, mark for skipping # But only if there are no other errors for this node if ( @@ -370,8 +396,9 @@ def make_node_credentials_input_map( """ result: dict[str, dict[str, JsonValue]] = {} - # Get aggregated credentials fields for the graph - graph_cred_inputs = graph.aggregate_credentials_inputs() + # Only map regular credentials (not auto_credentials, which are resolved + # at execution time from _credentials_id in file field data) + graph_cred_inputs = graph.regular_credentials_inputs for graph_input_name, (_, compatible_node_fields) in graph_cred_inputs.items(): # Best-effort map: skip missing items diff --git a/autogpt_platform/backend/backend/executor/utils_test.py b/autogpt_platform/backend/backend/executor/utils_test.py index db33249583..3a1fdfb9f9 100644 --- a/autogpt_platform/backend/backend/executor/utils_test.py +++ b/autogpt_platform/backend/backend/executor/utils_test.py @@ -907,3 +907,335 @@ async def test_stop_graph_execution_cascades_to_child_with_reviews( # Verify both parent and child status updates assert mock_execution_db.update_graph_execution_stats.call_count >= 1 + + +# ============================================================================ +# Tests for auto_credentials validation in _validate_node_input_credentials +# (Fix 3: SECRT-1772 + Fix 4: Path 4) +# ============================================================================ + + +@pytest.mark.asyncio +async def test_validate_node_input_credentials_auto_creds_valid( + mocker: MockerFixture, +): + """ + [SECRT-1772] When a node has auto_credentials with a valid _credentials_id + that exists in the store, validation should pass without errors. + """ + from backend.executor.utils import _validate_node_input_credentials + + mock_node = mocker.MagicMock() + mock_node.id = "node-with-auto-creds" + mock_node.credentials_optional = False + mock_node.input_default = { + "spreadsheet": { + "_credentials_id": "valid-cred-id", + "id": "file-123", + "name": "test.xlsx", + } + } + + mock_block = mocker.MagicMock() + # No regular credentials fields + mock_block.input_schema.get_credentials_fields.return_value = {} + # Has auto_credentials fields + mock_block.input_schema.get_auto_credentials_fields.return_value = { + "credentials": { + "field_name": "spreadsheet", + "config": {"provider": "google", "type": "oauth2"}, + } + } + mock_node.block = mock_block + + mock_graph = mocker.MagicMock() + mock_graph.nodes = [mock_node] + + # Mock the credentials store to return valid credentials + mock_store = mocker.MagicMock() + mock_creds = mocker.MagicMock() + mock_creds.id = "valid-cred-id" + mock_store.get_creds_by_id = mocker.AsyncMock(return_value=mock_creds) + mocker.patch( + "backend.executor.utils.get_integration_credentials_store", + return_value=mock_store, + ) + + errors, nodes_to_skip = await _validate_node_input_credentials( + graph=mock_graph, + user_id="test-user", + nodes_input_masks=None, + ) + + assert mock_node.id not in errors + assert mock_node.id not in nodes_to_skip + + +@pytest.mark.asyncio +async def test_validate_node_input_credentials_auto_creds_missing( + mocker: MockerFixture, +): + """ + [SECRT-1772] When a node has auto_credentials with a _credentials_id + that doesn't exist for the current user, validation should report an error. + """ + from backend.executor.utils import _validate_node_input_credentials + + mock_node = mocker.MagicMock() + mock_node.id = "node-with-bad-auto-creds" + mock_node.credentials_optional = False + mock_node.input_default = { + "spreadsheet": { + "_credentials_id": "other-users-cred-id", + "id": "file-123", + "name": "test.xlsx", + } + } + + mock_block = mocker.MagicMock() + mock_block.input_schema.get_credentials_fields.return_value = {} + mock_block.input_schema.get_auto_credentials_fields.return_value = { + "credentials": { + "field_name": "spreadsheet", + "config": {"provider": "google", "type": "oauth2"}, + } + } + mock_node.block = mock_block + + mock_graph = mocker.MagicMock() + mock_graph.nodes = [mock_node] + + # Mock the credentials store to return None (cred not found for this user) + mock_store = mocker.MagicMock() + mock_store.get_creds_by_id = mocker.AsyncMock(return_value=None) + mocker.patch( + "backend.executor.utils.get_integration_credentials_store", + return_value=mock_store, + ) + + errors, nodes_to_skip = await _validate_node_input_credentials( + graph=mock_graph, + user_id="different-user", + nodes_input_masks=None, + ) + + assert mock_node.id in errors + assert "spreadsheet" in errors[mock_node.id] + assert "not available" in errors[mock_node.id]["spreadsheet"].lower() + + +@pytest.mark.asyncio +async def test_validate_node_input_credentials_both_regular_and_auto( + mocker: MockerFixture, +): + """ + [SECRT-1772] A node that has BOTH regular credentials AND auto_credentials + should have both validated. + """ + from backend.executor.utils import _validate_node_input_credentials + + mock_node = mocker.MagicMock() + mock_node.id = "node-with-both-creds" + mock_node.credentials_optional = False + mock_node.input_default = { + "credentials": { + "id": "regular-cred-id", + "provider": "github", + "type": "api_key", + }, + "spreadsheet": { + "_credentials_id": "auto-cred-id", + "id": "file-123", + "name": "test.xlsx", + }, + } + + mock_credentials_field_type = mocker.MagicMock() + mock_credentials_meta = mocker.MagicMock() + mock_credentials_meta.id = "regular-cred-id" + mock_credentials_meta.provider = "github" + mock_credentials_meta.type = "api_key" + mock_credentials_field_type.model_validate.return_value = mock_credentials_meta + + mock_block = mocker.MagicMock() + # Regular credentials field + mock_block.input_schema.get_credentials_fields.return_value = { + "credentials": mock_credentials_field_type, + } + # Auto-credentials field + mock_block.input_schema.get_auto_credentials_fields.return_value = { + "auto_credentials": { + "field_name": "spreadsheet", + "config": {"provider": "google", "type": "oauth2"}, + } + } + mock_node.block = mock_block + + mock_graph = mocker.MagicMock() + mock_graph.nodes = [mock_node] + + # Mock the credentials store to return valid credentials for both + mock_store = mocker.MagicMock() + mock_regular_creds = mocker.MagicMock() + mock_regular_creds.id = "regular-cred-id" + mock_regular_creds.provider = "github" + mock_regular_creds.type = "api_key" + + mock_auto_creds = mocker.MagicMock() + mock_auto_creds.id = "auto-cred-id" + + def get_creds_side_effect(user_id, cred_id): + if cred_id == "regular-cred-id": + return mock_regular_creds + elif cred_id == "auto-cred-id": + return mock_auto_creds + return None + + mock_store.get_creds_by_id = mocker.AsyncMock(side_effect=get_creds_side_effect) + mocker.patch( + "backend.executor.utils.get_integration_credentials_store", + return_value=mock_store, + ) + + errors, nodes_to_skip = await _validate_node_input_credentials( + graph=mock_graph, + user_id="test-user", + nodes_input_masks=None, + ) + + # Both should validate successfully - no errors + assert mock_node.id not in errors + assert mock_node.id not in nodes_to_skip + + +@pytest.mark.asyncio +async def test_validate_node_input_credentials_auto_creds_skipped_when_none( + mocker: MockerFixture, +): + """ + When a node has auto_credentials but the field value has _credentials_id=None + (e.g., from upstream connection), validation should skip it without error. + """ + from backend.executor.utils import _validate_node_input_credentials + + mock_node = mocker.MagicMock() + mock_node.id = "node-with-chained-auto-creds" + mock_node.credentials_optional = False + mock_node.input_default = { + "spreadsheet": { + "_credentials_id": None, + "id": "file-123", + "name": "test.xlsx", + } + } + + mock_block = mocker.MagicMock() + mock_block.input_schema.get_credentials_fields.return_value = {} + mock_block.input_schema.get_auto_credentials_fields.return_value = { + "credentials": { + "field_name": "spreadsheet", + "config": {"provider": "google", "type": "oauth2"}, + } + } + mock_node.block = mock_block + + mock_graph = mocker.MagicMock() + mock_graph.nodes = [mock_node] + + errors, nodes_to_skip = await _validate_node_input_credentials( + graph=mock_graph, + user_id="test-user", + nodes_input_masks=None, + ) + + # No error - chained data with None cred_id is valid + assert mock_node.id not in errors + + +# ============================================================================ +# Tests for CredentialsFieldInfo auto_credential tag (Fix 4: Path 4) +# ============================================================================ + + +def test_credentials_field_info_auto_credential_tag(): + """ + [Path 4] CredentialsFieldInfo should support is_auto_credential and + input_field_name fields for distinguishing auto from regular credentials. + """ + from backend.data.model import CredentialsFieldInfo + + # Regular credential should have is_auto_credential=False by default + regular = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["github"], + "credentials_types": ["api_key"], + }, + by_alias=True, + ) + assert regular.is_auto_credential is False + assert regular.input_field_name is None + + # Auto credential should have is_auto_credential=True + auto = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["google"], + "credentials_types": ["oauth2"], + "is_auto_credential": True, + "input_field_name": "spreadsheet", + }, + by_alias=True, + ) + assert auto.is_auto_credential is True + assert auto.input_field_name == "spreadsheet" + + +def test_make_node_credentials_input_map_excludes_auto_creds( + mocker: MockerFixture, +): + """ + [Path 4] make_node_credentials_input_map should only include regular credentials, + not auto_credentials (which are resolved at execution time). + """ + from backend.data.model import CredentialsFieldInfo, CredentialsMetaInput + from backend.executor.utils import make_node_credentials_input_map + from backend.integrations.providers import ProviderName + + # Create a mock graph with aggregate_credentials_inputs that returns + # both regular and auto credentials + mock_graph = mocker.MagicMock() + + regular_field_info = CredentialsFieldInfo.model_validate( + { + "credentials_provider": ["github"], + "credentials_types": ["api_key"], + "is_auto_credential": False, + }, + by_alias=True, + ) + + # Mock regular_credentials_inputs property (auto_credentials are excluded) + mock_graph.regular_credentials_inputs = { + "github_creds": (regular_field_info, {("node-1", "credentials")}), + } + + graph_credentials_input = { + "github_creds": CredentialsMetaInput( + id="cred-123", + provider=ProviderName("github"), + type="api_key", + ), + } + + result = make_node_credentials_input_map(mock_graph, graph_credentials_input) + + # Regular credentials should be mapped + assert "node-1" in result + assert "credentials" in result["node-1"] + + # Auto credentials should NOT appear in the result + # (they would have been mapped to the kwarg_name "credentials" not "spreadsheet") + for node_id, fields in result.items(): + for field_name, value in fields.items(): + # Verify no auto-credential phantom entries + if isinstance(value, dict): + assert "_credentials_id" not in value diff --git a/autogpt_platform/frontend/src/components/contextual/GoogleDrivePicker/helpers.ts b/autogpt_platform/frontend/src/components/contextual/GoogleDrivePicker/helpers.ts index 05b591ebf6..ab7b92c20b 100644 --- a/autogpt_platform/frontend/src/components/contextual/GoogleDrivePicker/helpers.ts +++ b/autogpt_platform/frontend/src/components/contextual/GoogleDrivePicker/helpers.ts @@ -4,7 +4,9 @@ import { loadScript } from "@/services/scripts/scripts"; export async function loadGoogleAPIPicker(): Promise { validateWindow(); - await loadScript("https://apis.google.com/js/api.js"); + await loadScript("https://apis.google.com/js/api.js", { + referrerPolicy: "no-referrer-when-downgrade", + }); const googleAPI = window.gapi; if (!googleAPI) { @@ -27,7 +29,9 @@ export async function loadGoogleIdentityServices(): Promise { throw new Error("Google Identity Services cannot load on server"); } - await loadScript("https://accounts.google.com/gsi/client"); + await loadScript("https://accounts.google.com/gsi/client", { + referrerPolicy: "no-referrer-when-downgrade", + }); const google = window.google; if (!google?.accounts?.oauth2) {