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

feat(utils): implement symlink sandbox for cachi2 #2139

Conversation

ben-alkov
Copy link
Member

What/why: implement detection/removal of unsafe symlinks in repos, specifically
covering cachi2 use case: Cachito already does this

How:

  • copypasta _enforce_sandbox() and related unit tests from Cachito ("cachito/cachito/workers/tasks/general.py" and "cachito/tests/test_workers/test_tasks/test_general.py", respectively)
  • add call to _enforce_sandbox()
  • add CLI boolean arg remove-unsafe-symlinks, which toggles removing all symlinks which point to location(s) outside of any cloned repository

Signed-off-by: Ben Alkov [email protected]

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED

Maintainers will complete the following section

  • Commit messages are descriptive enough
  • Code coverage from testing does not decrease and new code is covered
  • Python type annotations added to new code
  • JSON/YAML configuration changes are updated in the relevant schema
  • Changes to metadata also update the documentation for the metadata
  • Pull request has a link to an osbs-docs PR for user documentation updates
  • New feature can be disabled from a configuration file

What/why: implement detection/removal of unsafe symlinks in repos, specifically
covering cachi2 use case: Cachito already does this

How:

- copypasta `_enforce_sandbox()` and related unit tests from Cachito
  ("cachito/cachito/workers/tasks/general.py" and
   "cachito/tests/test_workers/test_tasks/test_general.py", respectively)
- add call to `_enforce_sandbox()`
- add CLI boolean arg `remove-unsafe-symlinks`, which toggles removing all
  symlinks which point to location(s) outside of any cloned repository

Signed-off-by: Ben Alkov <[email protected]>

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
@MartinBasti
Copy link
Contributor

LGTM, just tests are failing for some reason

@MartinBasti
Copy link
Contributor

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED

why is this in commit message?

@@ -135,6 +135,7 @@ def process_remote_sources(self) -> List[Dict[str, Any]]:
remote_source_data["ref"]
)

enforce_sandbox(source_path_app, remove_unsafe_symlinks=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

will be removal of unsafe symlinks implemented in followup PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had intended it for this PR, but it can be in a followup if you want to get this merged sooner.

@ben-alkov
Copy link
Member Author

why is this in commit message?

It's a pre-commit hook to check for secrets.

@ben-alkov ben-alkov merged commit f7064c7 into containerbuildsystem:feature_cachi2 Dec 18, 2024
9 of 12 checks passed
@ben-alkov ben-alkov deleted the feat-utils-cachi2-symlink-sandbox branch December 18, 2024 19:10
@MartinBasti
Copy link
Contributor

why is this in commit message?

It's a pre-commit hook to check for secrets.

But why should a random local RH specific pre-commit be recorded in commits upstream?

@ben-alkov
Copy link
Member Author

why is this in commit message?

It's a pre-commit hook to check for secrets.

But why should a random local RH specific pre-commit be recorded in commits upstream?

Noted. Commit message disabled.

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.

2 participants