Bugs/fix_persistent_storage

[alexcos20]

Changes proposed in this PR:

• expose persistentStorage in node status
• allow config without node accesslist

[alexcos20]

/run-security-scan

[alexcos20]

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This pull request addresses a potential bug in persistent storage access list checks and enhances the node's status reporting. Specifically, it ensures that access list checks are only performed if accessLists are explicitly configured for persistent storage, preventing potential errors when accessLists might be null or undefined. Additionally, it updates the OceanNodeStatus interface and the status handler to expose configured persistent storage access lists in the node's status, improving observability. A new integration test is added to verify this status reporting functionality. The change from getConfiguration() to oceanNode.getConfig() in the status handler is a minor improvement for consistency and potentially performance.

Comments:
• [INFO][bug] This change correctly guards the checkAddressOnAccessList call, preventing potential errors or unexpected behavior if config.persistentStorage exists but accessLists is null or undefined. This is a good fix for the identified bug.
• [INFO][performance] Changing await getConfiguration() to oceanNode.getConfig() is a good refactor. It leverages the already loaded configuration from the oceanNode instance, avoiding a potentially redundant async call and ensuring consistency with the current node state.
• [INFO][other] Adding the persistentStorage.accessLists to the node status is a valuable enhancement for observability, allowing node operators to easily check the configured access lists. This aligns well with the new OceanNodeStatus interface definition.
• [INFO][other] The new integration test for exposing persistent storage access lists on node status provides good coverage for the added functionality. It correctly asserts the presence and content of the access lists in the status response.
• [INFO][style] The addition of persistentStorage?: { accessLists?: AccessList[] } to OceanNodeStatus is necessary for the new status reporting feature. Consider if accessLists should potentially be a readonly array if it's meant to be immutable configuration data.

[alexcos20]

/run-security-scan

[alexcos20]

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This pull request introduces an environment variable PERSISTENT_STORAGE to configure persistent storage access lists, making the access control an opt-in feature. It refines the PersistentStorageCreateBucketHandler to conditionally enforce access list checks only when persistentStorage.accessLists are explicitly configured. The node's status endpoint is updated to expose this persistent storage configuration, and a new integration test verifies this behavior. Additionally, it refactors the statusHandler to directly use oceanNode.getConfig() for consistency and removes an unnecessary import. Documentation for the new environment variable is also included.

Comments:
• [INFO][other] This change makes the access list check conditional on config.persistentStorage?.accessLists being present. This means if PERSISTENT_STORAGE env var is not set or doesn't define accessLists, then no access control will be applied for bucket creation. This is a reasonable design choice (opt-in security) but worth noting for clarity in configuration guides.
• [INFO][style] Good refactor to use oceanNode.getConfig() directly, removing the need for getConfiguration() which might have been fetching config from elsewhere or re-reading it, potentially leading to inconsistencies or performance overheads. This improves clarity and consistency.
• [INFO][other] Adding an integration test for the persistent storage access lists exposed via the node status endpoint is excellent. It ensures that this new piece of operational information is correctly populated and visible.

[alexcos20]

/run-security-scan

[alexcos20]

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This pull request introduces significant improvements and bug fixes for the persistent storage feature, focusing on configuration handling, security (access lists), and performance (file uploads). It ensures robust initialization of local file system storage, enhances the node status reporting, and makes configuration parsing more resilient to common environment variable formatting issues.

Comments:
• [INFO][other] The logic for checkAddressOnAccessList has been updated to only apply if config.persistentStorage?.accessLists exists and has elements. This implies that if no access lists are configured for persistent storage, any address is allowed to create buckets. Please confirm this is the intended default behavior, providing an 'open-by-default' approach unless explicit access lists are provided.
• [INFO][performance] Replacing getConfiguration() with oceanNode.getConfig() is a good refactor, as it avoids re-reading configuration from sources and directly uses the already loaded instance configuration. This improves efficiency and consistency.
• [INFO][performance] Removing readRawBody and directly streaming req into the handler is a significant performance and memory optimization for file uploads. This change prevents buffering the entire request body in memory, which is crucial for handling large files efficiently and preventing potential out-of-memory issues. Excellent change!
• [INFO][bug] The added fs.mkdirSync and fs.statSync with comprehensive error handling for the base folder and buckets subfolder greatly improves the robustness of the local file system storage initialization. This ensures the configured path is valid, accessible, and correctly set up before operations begin.
• [INFO][style] The recursive: true option was removed from fsp.mkdir(path) when creating a new bucket. This is safe and good, as the buckets parent directory is now ensured to exist recursively during the PersistentStorageLocalFS constructor's initialization.
• [INFO][other] The preprocess logic for the persistentStorage configuration is very well thought out. It robustly handles various common ways environment variables for JSON objects can be malformed (e.g., plain JSON string, double-encoded JSON, JSON with single quotes). This significantly improves the developer experience and resilience of the configuration parsing.