AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request introduces a significant refactor and enhancement to the credential checking mechanism within the Ocean Node, supporting multi-chain access lists and more flexible match_any/match_all strategies. It updates the @oceanprotocol/ddo-js dependency, modifies configuration schemas for C2D environments and admin access, and centralizes on-chain access list checks. The changes are extensive, impacting core compute, download handlers, and admin authentication. Comprehensive test updates are included to reflect the new logic.

Comments:
• [INFO][other] The change from [] to null for authorizedDecryptersList, allowedValidatorsList, authorizedPublishersList, and allowedAdminsList is a functional change. While it aligns with the updated schema that processes null explicitly, ensure that downstream code correctly handles null instead of an empty array where these lists are used. If null implies 'no restrictions' and [] implies 'no one authorized', then this is an important semantic shift.
• [WARNING][other] Updated @oceanprotocol/ddo-js from 0.1.4 to 0.2.0. This is a minor version bump, but the diff indicates significant breaking changes in how Credentials are processed (e.g., checkCredentials signature change, removal of areKnownCredentialTypes). Ensure all consuming code handles these changes correctly. It would be beneficial to highlight these breaking changes in the PR description.
• [WARNING][other] Type of accessLists changed from string[] to { [chainId: string]: string[] }. This is a significant breaking type change for any module interacting with ComputeAccessList. All code referencing accessLists property in ComputeAccessList must be updated to handle the new object structure, which now supports per-chain access lists.
• [INFO][style] Refactored KNOWN_CREDENTIALS_TYPES into an enum-like CREDENTIALS_TYPES object. This is a good improvement for type safety and code readability. Ensure all usages are updated consistently.
• [WARNING][other] Type of allowedAdmins changed from string[] to { addresses: string[]; accessLists: AccessListContract }. Similar to C2D access lists, this allows for more granular and multi-chain control over admin permissions. This is a breaking type change for any code consuming nodeStatus.allowedAdmins.
• [INFO][performance] The blockchain instance is now created and isNetworkReady() checked before the credential checks for DDO and service levels. This is necessary because checkCredentials now requires a signer from the blockchain instance. This ensures network readiness is verified early, which is good for error handling, but it means a blockchain connection is established and verified even if credentials might deny access purely based on off-chain rules. For performance, ensure this doesn't add significant overhead in scenarios where off-chain credential checks are often sufficient.
• [WARNING][bug] The checkCredentials function signature has changed. The old areKnownCredentialTypes check is removed, and checkCredentials now takes consumerAddress, credentials, and signer. This is a significant API change. All calls to checkCredentials throughout the codebase must be updated to match the new signature and logic. The old credentials as Credentials cast might mask potential issues if the type structure is subtly different due to the ddo-js update.
• [INFO][bug] The condition for validateAccess to return true if no access lists or addresses are specified has been updated to correctly handle the new access.accessLists structure (checking !access.accessLists || Object.keys(access.accessLists).length === 0). This is a critical fix to ensure open access when no specific restrictions are configured.
• [INFO][bug] Added a try-catch block around oceanNode.getC2DEngines().getAllEngines(). This is a good improvement for robustness, preventing the entire /status endpoint from failing if C2D cluster initialization encounters an error.
• [INFO][test] Added a new test case for downloadAssetWithCredentialsWithMatchAll that fails for the first consumer. This implies a change in the default behavior or interpretation of match_all for credentials. It's crucial to ensure this behavior is clearly documented and understood, as it might be a breaking change for DDOs that previously relied on a different matching strategy.
• [WARNING][bug] The unit tests for checkCredentials have been updated to reflect the new signature requiring a signer and the new AccessList credential structure. Specifically, the test for ACCESS_LIST now includes chainId and an accessList contract address. It's important that the mock signer or blockchain setup for unit tests is robust enough to simulate on-chain checks effectively, or that a clear distinction is made between purely off-chain and on-chain test scenarios.
• [BUG][other] Added nonce: currentNonce to the deployAccessListContract transaction. This is a crucial fix, as not specifying a nonce can lead to 'nonce too low' errors when multiple transactions are sent rapidly from the same signer in a test environment. Good catch.
• [INFO][other] New file accessList.ts introduced, centralizing the logic for checkAddressOnAccessList. This promotes modularity and reusability for on-chain access list checks. This is a good design decision.
• [WARNING][other] The getAdminAddresses function now returns an object { addresses: string[]; accessLists: any } instead of just string[]. This means admin access can now be defined by both direct addresses and multi-chain access list contracts. This is a breaking change for any consumer of getAdminAddresses and needs careful handling where it's called (e.g., validateAdminSignature).
• [WARNING][security] The validateAdminSignature logic has been completely rewritten to support the new allowedAdmins structure. It now uses checkSingleCredential for both direct addresses and access lists. Ensure the logic correctly implements the intended authorization policy, especially regarding how multiple addresses and accessLists entries are combined (e.g., OR vs AND logic for allowing access). The current implementation appears to be an 'OR' logic across addresses and access lists, which is common but should be explicitly confirmed.
• [INFO][other] The AccessListContractSchema has been significantly improved to preprocess input values. It now gracefully handles null, JSON strings, and non-object types by normalizing them to null before validation. This makes the configuration more robust to different input formats, which is excellent for user experience and preventing schema validation failures.
• [ERROR][bug] This file has been almost completely rewritten. The old findCredential, hasAddressMatchAllRule, and areKnownCredentialTypes functions have been removed, and the core checkCredentials logic has been replaced with a more comprehensive implementation that supports match_deny/match_allow strategies and delegates to checkSingleCredential for type-specific checks (including on-chain access list checks). Given the criticality of this module for access control, an extremely thorough review of the new logic, including all possible combinations of credential types, match rules, and empty/non-empty lists, is essential. Any subtle bug here could have significant security implications.
• [INFO][performance] The checkCredentialOnAccessList function now uses the new checkAddressOnAccessList. While the new modularity is good, be mindful that on-chain calls (like balanceOf) can be expensive in terms of latency. If a DDO has many access lists across multiple chains, this could impact the performance of credential checks. Consider caching strategies or optimizing the number of on-chain calls if performance becomes an issue.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request introduces a significant refactor to the credential and access list management within ocean-node, primarily driven by an upgrade to @oceanprotocol/ddo-js (0.2.0) and the need for more sophisticated on-chain access control. Key changes include a revised structure for defining access lists and allowed administrators (now supporting chain-specific contract addresses), a complete overhaul of the checkCredentials logic to support match_allow and match_deny strategies, and early blockchain initialization to facilitate on-chain credential checks. Configuration parsing for access lists has also been made more robust. Overall, the changes aim to enhance flexibility, security, and extensibility of credential validation.

Comments:
• [INFO][other] The change from [] (empty array) to null for various list configurations (e.g., authorizedDecryptersList, allowedValidatorsList, authorizedPublishersList, allowedAdminsList) aligns with the updated schemas and type definitions, which is good. Ensure that any code expecting an empty array now correctly handles null values.
• [WARNING][other] The dependency @oceanprotocol/ddo-js is updated from ^0.1.4 to ^0.2.0. This is a minor version bump, but the semantic versioning implies potential breaking changes or new features that could impact existing DDO handling. Please confirm that all changes introduced by [email protected] have been fully accounted for and tested within ocean-node.
• [INFO][style] Replacing the KNOWN_CREDENTIALS_TYPES array with an enum-like CREDENTIALS_TYPES object is a great improvement for code clarity, maintainability, and type safety. Good job!
• [INFO][performance] Moving blockchain initialization and isNetworkReady checks earlier in the initialize flow is a crucial change. This ensures that on-chain credential checks (e.g., for accessList types) can be performed without delays or race conditions. This should also provide earlier feedback for network issues.
• [INFO][performance] Similar to initialize.ts, moving blockchain initialization and isNetworkReady checks earlier in the startCompute flow is a crucial improvement for robust on-chain credential validation.
• [INFO][performance] Reordering credential checks to occur after blockchain initialization in the downloadHandler is essential. This allows the new checkCredentials function to correctly utilize the blockchain signer for on-chain credential types like accessList.
• [WARNING][bug] The z.preprocess logic for AccessListContractSchema is much more robust for handling various input types (null, JSON strings, objects). However, it's critical to ensure this preprocessing correctly handles all expected and unexpected inputs from configuration files (e.g., empty string, malformed JSON string, array) to avoid runtime errors or incorrect configuration parsing in different deployment scenarios. Comprehensive testing on this parsing is recommended.
• [ERROR][other] The src/utils/credentials.ts file has been completely rewritten. The new checkCredentials, evaluateCredentialList, and checkSingleCredential functions introduce fundamental changes to how credentials are processed, including support for match_allow and match_deny rules, and integration with on-chain access lists. This is the most critical part of the PR. Thorough testing of all combinations of allow/deny lists, match_any/match_all rules, and both address and accessList credential types is absolutely essential. Any missed edge cases here could have significant security implications.
• [INFO][style] Extracting checkAddressOnAccessList into its own utility file (src/utils/accessList.ts) is a good separation of concerns and improves modularity. This function now serves as the single point of truth for on-chain access list checks.
• [INFO][security] Refactoring validateAdminSignature to use the new checkSingleCredential and getAdminAddresses (which returns structured data) makes the admin validation logic more consistent and flexible, aligning with the new credential structure. This is a positive change for security and configurability of admin access.
• [INFO][testing] Updating the c2dClusters configuration in the compute integration test to match the new accessLists structure is correct. Ensure that all new credential scenarios, including various match_allow/match_deny combinations and accessList specifics across different chains, are adequately covered by unit and integration tests.
• [INFO][testing] The unit tests for credentials.ts have been updated to reflect the new function signatures and logic. It's good to see Blockchain and Signer initialized for unit tests. However, given the complete rewrite of the checkCredentials function, ensure that the unit test coverage is exhaustive for all possible inputs and credential configurations, especially those related to match_allow, match_deny, and accessList types.