[ansible/ovos] Make sure .local belongs to user #259
Conversation
WalkthroughThis pull request updates the OVOS installer Ansible playbook to refine directory management and backup operations. The modifications remove the Changes
Sequence Diagram(s)sequenceDiagram
participant I as OVOS Installer
participant DC as Directory Checker
participant BP as Backup Processor
participant DR as Directory Creator
participant TG as Template Generator
I->>DC: Check existence of backup directories
DC-->>I: Return existence status
I->>BP: Proceed with backup if directories exist and cleaning flag set
BP-->>I: Backup complete
I->>DR: Create directories (.local and hivemind based on profile)
DR-->>I: Confirm directory creation
I->>TG: Generate mycroft.conf using installation method
TG-->>I: Configuration file generated
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
ansible/roles/ovos_installer/tasks/ovos.yml (1)
66-68:⚠️ Potential issueFix logical condition in hivemind directory creation.
The condition
ovos_installer_profile == 'ovos' and ovos_installer_profile == 'server'will never be true as a variable cannot equal two different values simultaneously.- { "directory": "{{ ovos_installer_user_home }}/hivemind", - "status": "{{ 'false' if (ovos_installer_profile == 'ovos' and ovos_installer_profile == 'server') else 'true' }}", + "status": "{{ 'false' if (ovos_installer_profile == 'ovos' or ovos_installer_profile == 'server') else 'true' }}", }Apply the same fix to the other hivemind directory conditions at lines 71 and 75.
🧹 Nitpick comments (1)
ansible/roles/ovos_installer/tasks/ovos.yml (1)
4-14: Consider security and completeness of backup operations.Two suggestions for the backup configuration:
- The wildcard patterns (
*) in backup paths might miss hidden files. Consider using.*in addition to*.- The backup archive permissions (0755) are too permissive for potentially sensitive user configuration data.
ovos_directories_backup: - - "{{ ovos_installer_user_home }}/ovos/*" + - "{{ ovos_installer_user_home }}/ovos/{*,.*}" # Apply similar pattern to other paths... community.general.archive: path: "{{ ovos_directories_backup }}" dest: "{{ ovos_installer_user_home }}/ovos-backup.tar.gz" owner: "{{ ovos_installer_user }}" group: "{{ ovos_installer_group }}" - mode: "0755" + mode: "0600"Also applies to: 29-33
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ansible/roles/ovos_installer/tasks/ovos.yml(1 hunks)
🔇 Additional comments (2)
ansible/roles/ovos_installer/tasks/ovos.yml (2)
93-96: Validate existing .local directory permissions.The
.localdirectory is a standard user directory that might already exist. Changing its ownership could affect other applications.Consider adding a check for existing permissions before modification:
- name: Check .local directory permissions ansible.builtin.stat: path: "{{ ovos_installer_user_home }}/.local" register: local_dir_stat - name: Ensure .local directory exists with correct permissions ansible.builtin.file: path: "{{ ovos_installer_user_home }}/.local" owner: "{{ ovos_installer_user }}" group: "{{ ovos_installer_group }}" mode: "0755" state: directory when: - not local_dir_stat.stat.exists or local_dir_stat.stat.pw_name != ovos_installer_user
111-122: Configuration generation looks good!The configuration path selection and file permissions are appropriate. The backup flag provides safety for existing configurations.
Summary by CodeRabbit
Chores
Refactor