dissalow symlinks in local storages that point outside the datadir

[icewind1991]

For now it's always blocked, will look into adding a mount option seperatly

cc @PVince81

[mention-bot]

By analyzing the blame information on this pull request, we identified @DeepDiver1975, @PVince81 and @blizzz to be potential reviewers

[PVince81]

in which cases is this loop necessary ? broken symlinks ?

[icewind1991]

non existing files/folders, it checks the first existing parent folder for symlinks

[PVince81]

@icewind1991 from now on please mention @owncloud/filesystem for reviews in this part

[PVince81]

• BUG: occ files:scan --all aborts when finding symlink with a bogus error message "Home storage for user admin not writable"

[PVince81]

• BUG: "following symlink not allowed" exception is causing Webdav 500. Please make this a 403

[PVince81]

Not sure if a bug, but the disallowed symlink doesn't appear in Webdav's PROPFIND results.
This means it's invisible but a user can still perform operations, like mkdir symlink, trying to overwrite it somehow. Still gives a 500 like above.

[PVince81]

Bugs were found, setting back to "Developing"

[icewind1991]

@PVince81 all fixed

[PVince81]

@icewind1991 thanks, it works now.

Can you make it so that the invalid symlink still appears in the list ?
Currently it's invisible and it could cause UX trouble or other bugs when trying to perform operations on an invisible but forbidden file.

[icewind1991]

It's possible but will take more work since it will make the blocking of symlinks conditional.

I also don't think it's an important enough usecase

[PVince81]

Hmm, or it will need a different implementation. Basically let it always be displayed, but when trying to scan or access it, block it with 403 like the firewall does.

I'm ok with merging this as is as it's a corner case, but please let me know what you think about the above idea.

[icewind1991]

For it to be displayed it needs to allow scanning

[PVince81]

👍

[PVince81]

Needs second review @owncloud/filesystem @mmattel

[PVince81]

@nickvergessen @georgehrke

[PVince81]

@guruz @dragotin

[guruz]

Looks ok, haven't tested it.. 👍

[PVince81]

@icewind1991 can you rebase to kick CI ? Seems even sqlite died 💀

[icewind1991]

jenkins has been poked

[nickvergessen]

integration tests fail

[icewind1991]

Forgot to update the tests when I changed the exception, should be fine now

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.