- 
                Notifications
    You must be signed in to change notification settings 
- Fork 19
fix: unarchive fails for user id != 1000 #240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: unarchive fails for user id != 1000 #240
Conversation
80be532    to
    4da6829      
    Compare
  
    4da6829    to
    2c92b58      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where unarchiving fails when the paperless_ngx_conf_usermap_uid is set to a value different from 1000. The fix involves determining the correct username for user ID 1000 and using it as the become_user for unarchive operations.
- Adds logic to lookup the username for user ID 1000 when paperless_ngx_conf_usermap_uidis not 1000
- Updates unarchive tasks to use the determined username as become_user
- Modifies permission handling to work with the new unarchive approach
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description | 
|---|---|
| tasks/preparation/platform.yml | Adds username lookup logic for user ID 1000 based on paperless_ngx_conf_usermap_uidconfiguration | 
| tasks/paperless_ngx/release_files.yml | Updates unarchive process to use determined username and adjusts permission handling | 
| tasks/base_dependencies/python.yml | Applies the same unarchive fix for Python source downloads | 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| key: 1000 | ||
| split: ':' | ||
| register: _getent_query_result | ||
|  | 
    
      
    
      Copilot
AI
    
    
    
      Sep 2, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line assumes that the getent query will always return exactly one result with UID 1000. If no user with UID 1000 exists, accessing [0] will cause an IndexError. Add error handling or validation to ensure the query returns results before accessing the first element.
| when: (paperless_ngx_conf_usermap_uid is undefined) or paperless_ngx_conf_usermap_uid != "1000" | ||
|  | ||
| - name: Set fact username for user with id 1000 | ||
| ansible.builtin.set_fact: | ||
| _paperless_ngx_untar_username: "{{ paperless_ngx_system_user }}" | ||
| when: (paperless_ngx_conf_usermap_uid is defined) and paperless_ngx_conf_usermap_uid == "1000" | 
    
      
    
      Copilot
AI
    
    
    
      Sep 2, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comparison uses string \"1000\" in line 31 but integer comparison in line 26. This inconsistency could lead to unexpected behavior if paperless_ngx_conf_usermap_uid is defined as an integer. Use consistent data types for comparison, preferably converting to string or integer consistently.
| when: (paperless_ngx_conf_usermap_uid is undefined) or paperless_ngx_conf_usermap_uid != "1000" | |
| - name: Set fact username for user with id 1000 | |
| ansible.builtin.set_fact: | |
| _paperless_ngx_untar_username: "{{ paperless_ngx_system_user }}" | |
| when: (paperless_ngx_conf_usermap_uid is defined) and paperless_ngx_conf_usermap_uid == "1000" | |
| when: (paperless_ngx_conf_usermap_uid is undefined) or (paperless_ngx_conf_usermap_uid | string) != "1000" | |
| - name: Set fact username for user with id 1000 | |
| ansible.builtin.set_fact: | |
| _paperless_ngx_untar_username: "{{ paperless_ngx_system_user }}" | |
| when: (paperless_ngx_conf_usermap_uid is defined) and (paperless_ngx_conf_usermap_uid | string) == "1000" | 
| when: (paperless_ngx_conf_usermap_uid is undefined) or paperless_ngx_conf_usermap_uid != "1000" | ||
|  | ||
| - name: Set fact username for user with id 1000 | ||
| ansible.builtin.set_fact: | ||
| _paperless_ngx_untar_username: "{{ paperless_ngx_system_user }}" | ||
| when: (paperless_ngx_conf_usermap_uid is defined) and paperless_ngx_conf_usermap_uid == "1000" | 
    
      
    
      Copilot
AI
    
    
    
      Sep 2, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comparison uses string \"1000\" in line 31 but integer comparison in line 26. This inconsistency could lead to unexpected behavior if paperless_ngx_conf_usermap_uid is defined as an integer. Use consistent data types for comparison, preferably converting to string or integer consistently.
| when: (paperless_ngx_conf_usermap_uid is undefined) or paperless_ngx_conf_usermap_uid != "1000" | |
| - name: Set fact username for user with id 1000 | |
| ansible.builtin.set_fact: | |
| _paperless_ngx_untar_username: "{{ paperless_ngx_system_user }}" | |
| when: (paperless_ngx_conf_usermap_uid is defined) and paperless_ngx_conf_usermap_uid == "1000" | |
| when: (paperless_ngx_conf_usermap_uid is undefined) or (paperless_ngx_conf_usermap_uid | string) != "1000" | |
| - name: Set fact username for user with id 1000 | |
| ansible.builtin.set_fact: | |
| _paperless_ngx_untar_username: "{{ paperless_ngx_system_user }}" | |
| when: (paperless_ngx_conf_usermap_uid is defined) and (paperless_ngx_conf_usermap_uid | string) == "1000" | 
| {{ _install_tempdir_state.files[0].path }} | ||
| {%- endif -%} | ||
| - name: Pepare permissions of tmp dir for untaring paperless-ngx sources | 
    
      
    
      Copilot
AI
    
    
    
      Sep 2, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in task name: 'Pepare' should be 'Prepare' and 'untaring' should be 'untarring'.
| - name: Pepare permissions of tmp dir for untaring paperless-ngx sources | |
| - name: Prepare permissions of tmp dir for untarring paperless-ngx sources | 
| path: "{{ paperless_ngx_dependency_install_tmp_dir }}" | ||
| register: _python_archive_dir | ||
|  | ||
| - name: Pepare permissions of tmp dir for unarchiving python sources | 
    
      
    
      Copilot
AI
    
    
    
      Sep 2, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in task name: 'Pepare' should be 'Prepare'.
| - name: Pepare permissions of tmp dir for unarchiving python sources | |
| - name: Prepare permissions of tmp dir for unarchiving python sources | 
Refers to issue #239. If the
paperless_ngx_conf_usermap_uidvariable is set to an id different from 1000, the unarchive fails to unpack files and change their ownership. This seems to be because the user unpacking the archive is different from the one owning the files.This PR adds a lookup step to find out the correct user name with
id == 1000in causepaperless_ngx_conf_usermap_uid != 1000. The user name stored in the variable_paperless_ngx_untar_usernamewill then later be used as thebecome_userin the unarchive step.Note that with this solution
paperless_ngx_conf_usermap_uidis not definedthe lookup step will fail even though the user might perchance be created with the id 1000.
In order to prevent this one might to either have to move the system user creation step to an earlier stage or make
paperless_ngx_conf_usermap_uidmandatory. I was not sure if it was okay to go down any of these routes.