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

Protect against Git-repo jobs clobbering sys.modules #3859

Closed
2 of 12 tasks
glennmatthews opened this issue Jun 5, 2023 · 3 comments · Fixed by #3943
Closed
2 of 12 tasks

Protect against Git-repo jobs clobbering sys.modules #3859

glennmatthews opened this issue Jun 5, 2023 · 3 comments · Fixed by #3943
Assignees
Labels
type: bug Something isn't working as expected type: feature Introduction of new or enhanced functionality to the application
Milestone

Comments

@glennmatthews
Copy link
Contributor

As ...

Patti - Platform Admin

I want ...

To add Jobs via Git repositories without worrying about them impacting the overall stability of the platform.

After #3840 , there is a risk that a misnamed (or maliciously-named) Git repository could potentially clobber existing Python code. For example, creating a repository and assigning it the slug nautobot causes all sorts of havoc as it results in Nautobot unloading itself and then attempting to reimport all of Nautobot's code from the Git repository by that name.

So that ...

The application is resilient to user error as well as mischievous/malicious actions by authenticated users.

I know this is done when...

  • Creating a GitRepository whose slug matches any currently loaded Python module in the Nautobot environment is rejected with an appropriate error message.

Optional - Feature groups this request pertains to.

  • Automation
  • Circuits
  • DCIM
  • IPAM
  • Misc (including Data Sources)
  • Organization
  • Plugins (and other Extensibility)
  • Security (Secrets, etc)
  • Image Management
  • UI/UX
  • Documentation
  • Other (not directly a platform feature)

Database Changes

None

External Dependencies

None

@glennmatthews glennmatthews added the type: feature Introduction of new or enhanced functionality to the application label Jun 5, 2023
@glennmatthews glennmatthews added this to the v2.0.0 milestone Jun 5, 2023
@glennmatthews glennmatthews added the type: bug Something isn't working as expected label Jun 5, 2023
@glennmatthews
Copy link
Contributor Author

As @gsnider2195 pointed out, since we add GIT_ROOT to the end of sys.path, we should also ensure that the git repo slug isn't shadowed by any already-importable module that would come from an earlier sys.path entry (e.g. standard library packages like math or traceback). Possibly importlib.util.find_spec(gitrepo.slug) or similar would be an appropriate check.

@bryanculver
Copy link
Member

bryanculver commented Jun 12, 2023

Add check in clean (see others here:

def clean(self):
super().clean()
if self.slug != "":
check_if_key_is_graphql_safe(self.__class__.__name__, self.slug, "slug")
if self.present_in_database and self.slug != self.__initial_slug:
raise ValidationError(
f"Slug cannot be changed once set. Current slug is {self.__initial_slug}, "
f"requested slug is {self.slug}"
)
) implementing the above #3859 (comment)

@glennmatthews glennmatthews self-assigned this Jun 16, 2023
@glennmatthews glennmatthews linked a pull request Jun 16, 2023 that will close this issue
5 tasks
glennmatthews added a commit that referenced this issue Jun 21, 2023
* Fix #3859 - add validation of GitRepository.slug against Python modules

* Handle omitted slug case on initial create

* Adjust wording
@glennmatthews
Copy link
Contributor Author

Implemented by #3943.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't working as expected type: feature Introduction of new or enhanced functionality to the application
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants