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

Check broken symlinks and don't fail on them unnecessarily #1100

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

mkosiarc
Copy link
Contributor

@mkosiarc mkosiarc commented Jul 1, 2024

Previously, we were using the -f option for the readlink command. This means that if the symlink was broken (pointing to nonexistent file), the file path was not evaluated and the readlink command failed which meant that the git clone task failed as well.

By using the -m option, the symlink path will be evaluated every time. This means that we will not break builds that contain broken symlinks pointing to nonexistent files within the directory. However, if the symlink is pointing to nonexistent file OUTSIDE of the repo, we will fail the task, as expected to avoid security concerns.

STONEBLD-2492

@mkosiarc mkosiarc requested a review from a team July 1, 2024 08:41
@mmorhun
Copy link
Collaborator

mmorhun commented Jul 1, 2024

I think we should update other git-clone tasks.

Previously, we were using the -f option for the readlink command. This
means that if the symlink was broken (pointing to nonexistent file), the
file path was not evaluated and the readlink command failed which meant
that the git clone task failed as well.

By using the -m option, the symlink path will be evaluated every time.
This means that we will not break builds that contain broken symlinks
pointing to nonexistent files within the directory. However, if the
symlink is pointing to nonexistent file OUTSIDE of the repo, we will
fail the task, as expected to avoid security concerns.

STONEBLD-2492

Signed-off-by: mkosiarc <[email protected]>
@mkosiarc mkosiarc force-pushed the fix-symlink-check branch from 66b4d49 to f158342 Compare July 1, 2024 11:13
@mkosiarc mkosiarc added this pull request to the merge queue Jul 1, 2024
Merged via the queue into konflux-ci:main with commit 0d1223c Jul 1, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants