From 0918681da9b6e5b92b57cb382d9f3016ba9f20b9 Mon Sep 17 00:00:00 2001 From: Vanshaj Poonia Date: Wed, 1 Jul 2026 18:54:41 +0530 Subject: [PATCH 1/2] [fix] Ensure log files are owned by www-data #627 Log files in {{ openwisp2_path }}/log/ could be created as root, which prevented the application processes (uWSGI, celery, daphne, Django) running as www-data from writing to them. - Set the log directory owner to www_user (was root:www_group). - Pre-create the supervisor and Django log files with www_user:www_group ownership so supervisord (root) and Django do not create them as root. - Run load_initial_data as www_user so openwisp2.log is not created as root during deployment. - Install the cron jobs in the www-data crontab (and remove the obsolete root-owned entries) so log rollovers triggered by cron do not create root-owned files. Fixes #627 --- CHANGES.md | 8 ++++++++ tasks/cron.yml | 21 +++++++++++++++++++++ tasks/django.yml | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index cc4a5bdc..7dc6ef67 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,14 @@ Work in progress. +### Bugfixes + +- Ensured log files in the OpenWISP log directory (e.g. ``/opt/openwisp2/log/``) + are owned by the application user (``www-data``) instead of being created as + ``root``, which prevented the application processes (uWSGI, celery, daphne, + Django) from writing to them (`#627 + `_) + ## Version 25.10.2 [2026-01-28] # Bugfixes diff --git a/tasks/cron.yml b/tasks/cron.yml index 563f2d35..dfacd351 100644 --- a/tasks/cron.yml +++ b/tasks/cron.yml @@ -1,9 +1,14 @@ --- +# NOTE: these cron jobs run manage.py commands which use Django's logging and +# may create/rotate openwisp2.log. They are installed in the www-data user's +# crontab (user: "{{ www_user }}") so they run as the application user and do +# not create root-owned log files. - name: Install topology update cron when: openwisp2_network_topology become: true ansible.builtin.cron: name: "Update toplogies" + user: "{{ www_user }}" day: "{{ openwisp2_topology_update_frequency.day }}" hour: "{{ openwisp2_topology_update_frequency.hour }}" minute: "{{ openwisp2_topology_update_frequency.minute }}" @@ -14,6 +19,7 @@ become: true ansible.builtin.cron: name: "Save snapshots of topologies" + user: "{{ www_user }}" day: "{{ openwisp2_topology_save_snapshot_frequency.day }}" hour: "{{ openwisp2_topology_save_snapshot_frequency.hour }}" minute: "{{ openwisp2_topology_save_snapshot_frequency.minute }}" @@ -23,7 +29,22 @@ become: true ansible.builtin.cron: name: "clearsessions cronjob" + user: "{{ www_user }}" day: "*" hour: "04" minute: "30" job: "{{ virtualenv_path }}/bin/python {{ openwisp2_path }}/manage.py clearsessions" + +# Older versions of this role installed these cron jobs in root's crontab. +# Remove the stale root-owned entries so they don't keep running as root and +# creating root-owned log files after the upgrade. +- name: Remove obsolete root-owned cron jobs + become: true + ansible.builtin.cron: + name: "{{ item }}" + user: root + state: absent + loop: + - "Update toplogies" + - "Save snapshots of topologies" + - "clearsessions cronjob" diff --git a/tasks/django.yml b/tasks/django.yml index 90578c1e..83064218 100644 --- a/tasks/django.yml +++ b/tasks/django.yml @@ -17,12 +17,43 @@ ansible.builtin.file: path: "{{ openwisp2_path }}/log" state: directory + owner: "{{ www_user }}" group: "{{ www_group }}" mode: "0770" recurse: true tags: - molecule-idempotence-notest +# Supervisord and Django (RotatingFileHandler) both create their log files on +# first run. Supervisord runs as root and would otherwise create these files as +# root:root, and Django tasks running as root would do the same for +# openwisp2.log. Pre-creating the files with the correct ownership ensures the +# www-data processes (uWSGI, celery, daphne, Django) can always write to them. +- name: Ensure openwisp2 log files are owned by "{{ www_user }}" + ansible.builtin.file: + path: "{{ openwisp2_path }}/log/{{ item }}" + state: touch + owner: "{{ www_user }}" + group: "{{ www_group }}" + mode: "0640" + # keep the task idempotent (touch reports "changed" otherwise) + access_time: preserve + modification_time: preserve + loop: + - openwisp2.log + - uwsgi.log + - celery.log + - celerybeat.log + - celery-network.log + - celery-monitoring.log + - celery-firmware-upgrader.log + - daphne.log + # Services (e.g. supervisord running as root) may rotate/recreate these log + # files between converge runs, so skip the idempotence check for this task, + # consistent with the log directory task above. + tags: + - molecule-idempotence-notest + - name: Create custom static directory ansible.builtin.file: path: "{{ openwisp2_path }}/static_custom" @@ -224,6 +255,10 @@ mode: "0754" - name: Load initial data + # Run as the application user so that any log file (e.g. openwisp2.log) + # touched by Django during startup is not created as root. + become: true + become_user: "{{ www_user }}" environment: PRIVATE_KEY: "{{ default_private_ssh_key.stdout | default(None) }}" PUBLIC_KEY: "{{ default_public_ssh_key.stdout | default(None) }}" From a86863dd15bc9ead9a449e70a39f915ef8be2c7c Mon Sep 17 00:00:00 2001 From: Vanshaj Poonia Date: Wed, 1 Jul 2026 23:53:50 +0530 Subject: [PATCH 2/2] [fix] Address review feedback for log ownership fix #627 - Rewrote the CHANGES.md entry using Markdown link syntax so the issue reference renders as a clickable link. - Pre-create only the log files for the services that are actually enabled, matching the conditions used to install the supervisor programs, instead of creating empty files for disabled features. - Added regression tests to molecule verify that assert the log directory and supervisor/Django log files are owned by www-data and that the cron jobs run as www-data and not as root. --- CHANGES.md | 10 ++--- molecule/resources/verify.yml | 79 +++++++++++++++++++++++++++++++++++ tasks/django.yml | 24 +++++++---- 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 7dc6ef67..932f4951 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,11 +6,11 @@ Work in progress. ### Bugfixes -- Ensured log files in the OpenWISP log directory (e.g. ``/opt/openwisp2/log/``) - are owned by the application user (``www-data``) instead of being created as - ``root``, which prevented the application processes (uWSGI, celery, daphne, - Django) from writing to them (`#627 - `_) +- Ensured log files in the OpenWISP log directory (e.g. `/opt/openwisp2/log/`) + are owned by the application user (`www-data`) instead of being created as + `root`, which prevented the application processes (uWSGI, celery, daphne, + Django) from writing to them + ([#627](https://github.com/openwisp/ansible-openwisp2/issues/627)) ## Version 25.10.2 [2026-01-28] diff --git a/molecule/resources/verify.yml b/molecule/resources/verify.yml index 52727d41..5ed7ac60 100644 --- a/molecule/resources/verify.yml +++ b/molecule/resources/verify.yml @@ -60,6 +60,85 @@ debug: var: openwisp_log + # Regression tests for + # https://github.com/openwisp/ansible-openwisp2/issues/627 + # The log directory and the supervisor/Django log files must be owned by the + # application user (www-data) so the services running as www-data can write + # to them. (nginx*.log are intentionally excluded: they are created by the + # nginx master process running as root.) + - name: Get ownership of the OpenWISP log directory + ansible.builtin.stat: + path: /opt/openwisp2/log + register: openwisp2_log_dir + + - name: Ensure the OpenWISP log directory is owned by www-data + ansible.builtin.assert: + that: + - openwisp2_log_dir.stat.exists + - openwisp2_log_dir.stat.isdir + - openwisp2_log_dir.stat.pw_name == 'www-data' + - openwisp2_log_dir.stat.gr_name == 'www-data' + fail_msg: >- + /opt/openwisp2/log is not owned by www-data:www-data + (owner={{ openwisp2_log_dir.stat.pw_name | default('?') }}:{{ + openwisp2_log_dir.stat.gr_name | default('?') }}) + quiet: true + + - name: Get ownership of the OpenWISP log files + ansible.builtin.stat: + path: "/opt/openwisp2/log/{{ item }}" + loop: + - openwisp2.log + - uwsgi.log + - celery.log + - celerybeat.log + - celery-network.log + - celery-monitoring.log + - celery-firmware-upgrader.log + - daphne.log + register: openwisp2_log_files + + - name: Ensure OpenWISP log files exist and are owned by www-data + ansible.builtin.assert: + that: + - item.stat.exists + - item.stat.pw_name == 'www-data' + - item.stat.gr_name == 'www-data' + fail_msg: >- + {{ item.item }} is missing or not owned by www-data:www-data + (exists={{ item.stat.exists }}, owner={{ + item.stat.pw_name | default('?') }}:{{ + item.stat.gr_name | default('?') }}) + quiet: true + loop: "{{ openwisp2_log_files.results }}" + loop_control: + label: "{{ item.item }}" + + - name: Read the www-data crontab + ansible.builtin.command: crontab -u www-data -l + register: www_data_crontab + changed_when: false + + - name: Read the root crontab + ansible.builtin.command: crontab -u root -l + register: root_crontab + changed_when: false + failed_when: false + + - name: Ensure OpenWISP cron jobs run as www-data and not as root + ansible.builtin.assert: + that: + - "'manage.py clearsessions' in www_data_crontab.stdout" + - "'manage.py update_topology' in www_data_crontab.stdout" + - "'manage.py save_snapshot' in www_data_crontab.stdout" + - "'manage.py clearsessions' not in root_crontab.stdout" + - "'manage.py update_topology' not in root_crontab.stdout" + - "'manage.py save_snapshot' not in root_crontab.stdout" + fail_msg: >- + OpenWISP cron jobs are not installed in the www-data crontab as + expected (www-data crontab: {{ www_data_crontab.stdout }}) + quiet: true + - name: Check if FreeRADIUS is listening on WPA Enterprise site ports shell: "netstat -tuln | grep -Eq '1822|1823|18230'" register: freeradius_eap_ports # Register the output and return code diff --git a/tasks/django.yml b/tasks/django.yml index 83064218..2c8a21b5 100644 --- a/tasks/django.yml +++ b/tasks/django.yml @@ -39,15 +39,21 @@ # keep the task idempotent (touch reports "changed" otherwise) access_time: preserve modification_time: preserve - loop: - - openwisp2.log - - uwsgi.log - - celery.log - - celerybeat.log - - celery-network.log - - celery-monitoring.log - - celery-firmware-upgrader.log - - daphne.log + # Only pre-create the log files for the services that are actually enabled, + # matching the conditions used to install the supervisor programs in + # supervisor.yml, so we don't leave empty log files for disabled features. + loop: >- + {{ + ['openwisp2.log', 'uwsgi.log', 'celery.log'] + + (['celerybeat.log'] if openwisp2_celerybeat else []) + + (['celery-network.log'] if openwisp2_celery_network else []) + + (['celery-monitoring.log'] + if (openwisp2_monitoring and openwisp2_celery_monitoring) else []) + + (['celery-firmware-upgrader.log'] + if (openwisp2_firmware_upgrader and openwisp2_celery_firmware_upgrader) + else []) + + (['daphne.log'] if openwisp2_daphne_install else []) + }} # Services (e.g. supervisord running as root) may rotate/recreate these log # files between converge runs, so skip the idempotence check for this task, # consistent with the log directory task above.