Conversation
|
/run-security-scan |
alexcos20
left a comment
There was a problem hiding this comment.
AI automated code review (Gemini 3).
Overall risk: medium
Summary:
This pull request introduces a new validateDDO hook for the Policy Server, allowing external policy services to validate DDOs before they are processed by the Ocean Node. This enhances the flexibility and potential security posture for DDO management. The changes include updates to documentation, type definitions, the DDO handler, and the Policy Server component itself.
Comments:
• [INFO][performance] The PolicyServer is instantiated inside the handle method for every ValidateDDOCommand. While the constructor might be lightweight, consider if a singleton pattern or passing an already initialized instance could be more efficient, especially if the PolicyServer constructor eventually performs more resource-intensive operations (e.g., complex configuration loading). For now, this is likely acceptable, but it's a pattern to be mindful of for future optimizations.
• [INFO][style] The policyServer property is typed as any. While this offers maximum flexibility for passing arbitrary data to the policy server, it reduces type safety. If there's a common or expected structure for this object, consider defining a more specific interface (e.g., PolicyServerConfig or PolicyServerOptions) to improve readability and maintainability, even if it's just for documentation purposes within the code. If the structure is entirely external and unknown, any is a practical choice, but good to acknowledge.
• [INFO][style] The indentation for the initialize command example seems to have been slightly altered (two spaces instead of four). While minor, consistency in markdown formatting is good. Please ensure this aligns with the project's documentation style guide.
Add Policy Server check for validateDDO
Summary
When the Policy Server is configured, DDO validation now calls the policy server before returning a validation result. If the policy server denies the request, validation fails with 403.
Changes
1.
ValidateDDOcommand type (src/@types/commands.ts)ValidateDDOCommandpolicyServer?: anyso the validateDDO command can pass policy-server context (same pattern as other policy-backed commands).2. DDO validation handler (
src/components/core/handler/ddoHandler.ts)After existing validation logic (and before returning success):
isPolicyServerConfigured()is true:PolicyServerand callspolicyServer.validateDDO(task.ddo, task.publisherAddress, task.policyServer).3. Policy Server client (
src/components/policyServer/index.ts)validateDDO(rawDDO, publisherAddress, policyServer)validateDDOaction to the policy server withrawDDO,publisherAddress, andpolicyServerpayload.checkEncrypt, etc.).4. Documentation (
docs/PolicyServer.md)validateDDOactionaction: "validateDDO"rawDDOpublisherAddresspolicyServerinitializeaction (indentation/formatting only).Behavior
Policy server not configured
validateDDO behaves as before; no policy call is made.
Policy server configured
After local validation passes, the node calls the policy server with the DDO and publisher. If the policy server denies, the client gets 403 and no validation signature; if it allows, the existing success response is returned.