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

Carefully handle symlinks at the last phase of VerifyFilePromise() #5444

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

vpodzime
Copy link
Contributor

@vpodzime vpodzime commented Feb 8, 2024

If the promiser is a symlink, lstat() and stat() give different results. We need to use lstat() to get info about the promiser (symlink) itself.

Also, we need to call VerifyFileLeaf() on symlinks as well as on regular files.

Ticket: ENT-11235
Changelog: None

olehermanse
olehermanse previously approved these changes Feb 8, 2024
Copy link
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks.

@olehermanse
Copy link
Member

@cf-bottom please test this in Jenkins :)

@cf-bottom
Copy link

@vpodzime vpodzime force-pushed the master-symlink_perms branch from 06b58cd to 20c21cb Compare February 8, 2024 14:59
larsewi
larsewi previously approved these changes Feb 8, 2024
@vpodzime vpodzime dismissed stale reviews from larsewi and olehermanse via 9dd093e February 8, 2024 15:12
@vpodzime
Copy link
Contributor Author

vpodzime commented Feb 8, 2024

Sure, I triggered a build:

Build Status

Jenkins: https://ci.cfengine.com/job/pr-pipeline/10429/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10429/

Some acceptance tests need adjusting. Hopefully that's good news and the adapted tests will serve as tests for the new/right behavior.

If the promiser is a symlink, `lstat()` and `stat()` give
different results. We need to use `lstat()` to get info about the
promiser (symlink) itself.

Also, we need to call `VerifyFileLeaf()` on symlinks as well as
on regular files.

Ticket: ENT-11235
Changelog: Ownership of symlinks is now handled properly
Setting permissions on symlinks should not act on targets.

Ticket: CFE-4148
Changelog: None
@vpodzime vpodzime force-pushed the master-symlink_perms branch from 9dd093e to b267435 Compare February 9, 2024 08:19
@vpodzime
Copy link
Contributor Author

vpodzime commented Feb 9, 2024

@cf-bottom jenkins with exotics, please

@cf-bottom
Copy link

@vpodzime
Copy link
Contributor Author

Alright, I triggered a build:

Build Status

All green, just stuck in Jenkins and with an unrelated win-acceptance tests failure.

@vpodzime vpodzime merged commit 99b942d into cfengine:master Feb 12, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants