NiklasRosenstein / flux-ci

Flux is your own private & lightweight CI server.
MIT License
26 stars 10 forks source link

UI for Overrides #54

Closed tvrzna closed 6 years ago

tvrzna commented 6 years ago

References #31.

In Repository view page in was added menu item Overrides. In first visit it creates folder for overrides and allows to list it.

In every listed folder there is possibility to create new folder, create new empty file or upload files into current folder. Every file and folder could be deleted and renamed, for files there are two more options: first is simple text edit, it means it won't show images, but allows to change e.g. build script, README, ... Second is download the file.

As I already mentioned in issue, previous commits were rebased so it also contains latest changes from master.

In this state it is far from being perfect, but I believe that further development requires insight from more people.

NiklasRosenstein commented 6 years ago

Please avoid plain except: clauses. os.mknod() does not exist on Windows, and that error was caught and ignored. I've added some logging for now, at least.

NiklasRosenstein commented 6 years ago

The path sanitization needs to be a bit more sophisticated, I can for example use ..\foo.txt on Windows to create a file outside the designated override directory, and also read its contents. But the rest looks good! :)

image

tvrzna commented 6 years ago

Thank you for review and fixes :) When I get back to my computer, I'll use secure_filename method from flask (as it is used for uploaded files), it should be good enough.

NiklasRosenstein commented 6 years ago

I was going to suggest that function, but it is for filenames not paths, so any slashes will be converted to underscores. What you could do instead is to construct the absolute path, and then check if it is inside the allowed directory.

tvrzna commented 6 years ago

Before file and folder are created, there is just their "new" name and it could be sanitized by that function.

NiklasRosenstein commented 6 years ago

My mistake, I somehow thought the "name" could be a path but it is supposed to be a plain filename.

gsantner commented 6 years ago

is it ready to merge? :D

NiklasRosenstein commented 6 years ago

Sorry for the delay, yes I think this is fine now. :)