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 that all symlinks point to something existing within the problem package #261

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

gkreitz
Copy link
Contributor

@gkreitz gkreitz commented May 24, 2024

We recently saw issues caused by dangling symlinks being present in a problem package. This PR makes it an error to have any symlinks anywhere in the problem package point to something which does not exist, or point somewhere outside of the problem package.

Example output:

Loading problem hello
ERROR in hello: Symlink submissions/foo links to /home which is outside of problem package
ERROR in hello: Symlink submissions/foo links to /home which is an absolute path. Symlinks must be relative.
ERROR in hello: Symlink submissions/accepted/hello2.cc links to helloooooo.cc which does not exist
ERROR in hello: Symlink data/oops links to ../../data/sample which does not exist
ERROR in hello: Symlink data/oops links to ../../data/sample which is outside of problem package
...

@niemela
Copy link
Member

niemela commented May 24, 2024

Should we mention this in the spec? Or is it obvious that symlinks must point to something and that it mustn't point outside of the package?

@simonlindholm
Copy link
Member

It's not obvious that symlinks are allowed, so we may want to specify that. If we do, it certainly doesn't hurt to say that they must point to within the problem package.

@niemela
Copy link
Member

niemela commented May 24, 2024

It's not obvious that symlinks are allowed

That's a very good point. (And the rest follows).

@Tagl
Copy link
Contributor

Tagl commented May 24, 2024 via email

@gkreitz
Copy link
Contributor Author

gkreitz commented May 25, 2024

Well do we want to allow symlinks to point to absolute paths? That seems not so great me

Excellent point. Absolute symlinks, even if they point to the right place on the machine we're currently running on feel like a very bad idea in a problem package. I added another check for absolute symlinks.

@gkreitz gkreitz force-pushed the check_bad_symlinks branch from a389378 to 0ced1ad Compare May 25, 2024 08:43
@pehrsoderman pehrsoderman merged commit 9760867 into Kattis:develop Jun 6, 2024
3 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.

5 participants