Kattis / problemtools

Tools to manage problem packages using the Kattis problem package format.
MIT License
101 stars 70 forks source link

Check that all symlinks point to something existing within the problem package #261

Closed gkreitz closed 3 weeks ago

gkreitz commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

It's not obvious that symlinks are allowed

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

Tagl commented 1 month ago

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

On Fri, May 24, 2024, 19:34 Fredrik Niemelä @.***> wrote:

It's not obvious that symlinks are allowed

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

— Reply to this email directly, view it on GitHub https://github.com/Kattis/problemtools/pull/261#issuecomment-2130238033, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2ZBKUENQQAI5HVFEPNQG3ZD6I6BAVCNFSM6AAAAABIIDKQK6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZQGIZTQMBTGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

gkreitz commented 1 month ago

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.