Pull request for issue #10

[google-labs-jules]

Fixes #10

[bosd]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a new Odoo module, odf_core, which provides the basic framework for a data flow tool. It adds models for connections and flow projects, along with UI components and a scheduled action to execute the flows.

My review has identified several critical and high-severity issues that should be addressed:

• Security: Passwords are being stored in plaintext, the cron job runs as the root user, and access controls are too permissive. These issues pose significant security risks.
• Correctness: The use of manual commit() calls within a cron job breaks Odoo's transaction management, which can lead to data inconsistency.

I have provided detailed comments and suggestions for each of these points. Addressing them will greatly improve the security and robustness of the module.

[gemini-code-assist]

Storing passwords in plaintext in the database is a major security vulnerability. The password=True attribute only masks the password in the user interface but does not provide any encryption for storage. Anyone with database access can read these passwords.

You should use a secure method for storing credentials. Consider using Odoo's ir.config_parameter for sensitive data, which can be protected, or integrate with a secrets management tool like HashiCorp Vault. If you must store it in this model, the password should be encrypted before being stored and decrypted when used.

[gemini-code-assist]

The use of self.env.cr.commit() inside a loop is highly discouraged in Odoo as it breaks transaction atomicity. This can lead to data inconsistencies that are hard to debug. For instance, if the server crashes after a commit, the project's state could be left as 'running' incorrectly. Odoo cron jobs run in a single transaction, ensuring that all changes are rolled back if any part of the job fails, which maintains data integrity.

If you need to process each project in an isolated transaction, the recommended approach in Odoo is to use a job queue module (like queue_job from the OCA).

I recommend refactoring this method to remove the manual commits and wrap the logic for a single project in a try...except block. This ensures that the entire cron run is atomic.

        for project in projects:
            try:
                project.write({'status': 'running'})

                if not run_workflow:
                    raise ImportError("The 'odoo-data-flow' library is not available.")

                if not project.flow_file_path:
                    raise ValueError(f"Flow file path is not set for project '{project.name}'")

                # The odoo-data-flow library expects a path to a YAML config file.
                run_workflow(project.flow_file_path)

                project.write({'status': 'done'})
            except Exception:
                _logger.error(
                    "Failed to run data flow project '%s'.\n%s",
                    project.name,
                    traceback.format_exc()
                )
                project.write({'status': 'failed'})

[gemini-code-assist]

The scheduled action is configured to run as the root user (base.user_root), which has maximum privileges. This is a security risk. It's best practice to run cron jobs with a dedicated user that has only the minimum required permissions to perform its tasks.

[gemini-code-assist]

The current access control rules grant all internal users (base.group_user) full create, read, update, and delete (CRUD) permissions on odf.connection and odf.flow.project models. This is likely too permissive. Connection details, which may contain sensitive information, and the ability to run data flows should be restricted to authorized users.

I suggest restricting access to a more privileged group, for example, system administrators (base.group_system). For more granular control, you could create a dedicated security group (e.g., group_odf_manager) and assign access to that group instead.

access_odf_connection,odf.connection access,model_odf_connection,base.group_system,1,1,1,1
access_odf_flow_project,odf.flow.project access,model_odf_flow_project,base.group_system,1,1,1,1

[gemini-code-assist]

Catching a broad Exception can hide unexpected errors and make debugging more difficult. It's better practice to catch more specific exceptions that you anticipate, such as ImportError and ValueError which you raise explicitly. If run_workflow can raise other known exceptions, they should also be caught specifically. This makes the error handling more robust and the code's intent clearer.