Skip to content
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

zypper.CreateTarball() creates backup directory path elements with no permissions #232

Open
rtamalin opened this issue May 1, 2024 · 1 comment
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@rtamalin
Copy link
Collaborator

rtamalin commented May 1, 2024

While implementing unit tests for PR #229 I noticed that when the CreateTarball() routine creates the var/adm/backup/system-upgrade directory path, in which it will store the backup tarball and restore script, it is specifying no permissions, so the resulting directory path can only be accessed with root privileges.

Given that this code will normally be run as root, this hasn't been a problem up to now, but causes problems for the unit testing.

Using a reasonable permission, e.g. 0o700, that permits at least user access seems like a better approach.

@rtamalin rtamalin added the bug Something isn't working label May 1, 2024
@rtamalin rtamalin self-assigned this May 1, 2024
rtamalin added a commit that referenced this issue May 1, 2024
The Distribution Migration System (DMS) implements a non-interactive
fully automated offline migration of systems by booting into a ISO
image that runs through the migration. In some cases, if a migration
from one product stream to another product stream fails, the normal
rollback handling doesn't always restore the products state correctly.

To address this we should include the /etc/products.d contents in the
backup that is created and restored.

Add changelog entry for upcoming v1.9.0 release stream, including the
bug tracking id.

Add unit test for the zypper Backup and Restore routines.

Also fix two bugs found while developing the unit test:

  * CreateTarball()'s check for potential tar arguments incorrectly
    checked under the local file system hierarchy, rather than under
    the specified root hierarchy, meaning it could skip backing up
    entries that might exist under the root if they didn't exist
    locally.

  * The /var/adm/backup/system-upgrade directory created under the
    specified root hierarchy, and missing intermediary directories,
    were being created with no access permissions, meaning that only
    a privileged superuser could access them. In reality migrations
    are run as root so it didn't impact customer usage, but it did
    cause problems for the unit tests running as a non-privileged
    user.

Fixes: #231, #232

Co-authored-by: Felix Schnizlein <[email protected]>
@felixsch felixsch added the help wanted Extra attention is needed label Jun 19, 2024
@rtamalin
Copy link
Collaborator Author

rtamalin commented Aug 7, 2024

@felixsch if a permission of 0o700 is acceptable, that was implemented in #229 so this issue can probably be closed as fixed.

@felixsch felixsch added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants