Crash with "getDirectoryContents:openDirStream: permission denied (Permission denied)"

[wrengr]

The Hakyll.Core.Util.File.getRecursiveContents function does not check for permissions before it calls System.Directory.getDirectoryContents. Therefore, when encountering directories that it does not have permissions for, it will crash the program with the aforementioned error message.

In particular, this happens when Hakyll.Core.Runtime.run calls Hakyll.Core.Provider.Internal.newProvider which in turn calls getRecursiveContents. Which is to say, every time we start up our hakyll program it will try to read every single thing underneath the current directory — including things like cabal's dist-newstyle directory, darcs's _darcs directory, and hakyll's own _cache and _site directories! This is horribly inefficient, not to mention a security/correctness problem. Those specific directories have strange names, so it's easy enough to filter them out by setting the shouldIgnoreFile field of the Configuration. However, because shouldIgnoreFile can only look at the basename of each file, it cannot filter out all sorts of other directories the user might want to keep hakyll out of (e.g., I like to have top-level directories with names like 'TODO' or 'BUGS', so I can keep track of notes-to-self; but then maybe I also want my website to have directories with those names). Therefore, it would also be nice to have some mechanism for filtering out directories based on the complete path (relative to top); or even better would be to make it a whitelist instead of a blacklist/filter.

---

I discovered this crash when trying to work around the crash reported in #1061. I was hoping to triage that crash by simply removing permissions to the directory where the broken symlinks live, which uncovered the current crash.

These two bugs might be more related that it appears on the surface. That is, if System.Directory.doesDirectoryExist returning false is ever interpreted to mean the thing is a (standard) file, then that's a problem: because it returns false for broken symlinks. I'm not sure if that mistake is actually made anywhere in the codebase, but the two crashes make me wonder how posixly-correct the rest of the file manipulation code is.

[LaurentRDC]

There are a few things in this issue.

First, the cause of the crash: it is true that permissions are not checked; all files in a directory are crawled and returned.

if System.Directory.doesDirectoryExist returning false is ever interpreted to mean the thing is a (standard) file, then that's a problem

Unfortunately, this is the case. So, there's room for improvement here.

Note that run does not necessarily crawl all files by default. There is an ability to ignore files (such as in directories whose names start with '.'. In particular, many directories you mentioned, e.g. _site, are not crawled by default because, as you say, this would be inefficient. But other directories are crawled by default, such as dist-newstyle, so again there is room for improvement.

Indeed, the ignoreFile filter in the configuration is only passed basenames, which is annoying. I wonder how we can plug this hole without breaking backwards compatibility.

In the meantime, note that you can place all files required for compilation of the website into a directory (e.g. web-src), and use this as the provider directory (which is '.' by default).

[LaurentRDC]

I have added a check for permissions in bdfbead, but the other issues brought up in this issue remain

[wrengr]

if System.Directory.doesDirectoryExist returning false is ever interpreted to mean the thing is a (standard) file, then that's a problem

Unfortunately, this is the case. So, there's room for improvement here.

Yeah, that particular instance is easy enough to fix (I can submit a PR tomorrow if nobody's done it yet by then). I was more worried about whether there're other places where that sort of logic mistake might be lurking. It's easy enough to grep for other calls to the directory functions; but I'm not particularly familiar with the implementation details of providers and stores, to know if there might be a subtler issue afoot.

In particular, many directories you mentioned, e.g. _site, are not crawled by default

Aha, I missed that :) Well, at least those directories aren't a worry

Indeed, the ignoreFile filter in the configuration is only passed basenames, which is annoying. I wonder how we can plug this hole without breaking backwards compatibility.

I have a few different ideas, depending on how dynamic vs static-analysis, and how explicit vs automatic we want things to be.

For a more static-analysis sort of solution, we could reuse the Pattern type to construct the pattern to use for filtering. This would be nice since we already have a library of helper functions for manipulating these patterns. Plus, it could enable automatic discovery of what the pattern should be: (1) assume that anything the user explicitly mentions in their rules is fair game, (2) use runRules to compile the pattern of everything they match against.[1] Unfortunately, I'm guessing this won't actually work out because of the ways that runRules relies on already having the provider, etc. Or at least, last time I tried looking into how to do some static analysis of this sort, it didn't seem feasible.[2]

For the more dynamic solution, just add another Configuration field of type FilePath->Bool where the argument is the complete relative path. And behind the scenes the old shouldIgnore can be automatically converted to the new filter (via a basename combinator). Although this is easy to implement, I don't think it'd end up being very nice to work with. One reason why is because the two filters are kinda working at cross purposes: the old shouldIgnore is a blacklist, since we assume there's only a small well-structured set of things we want to ignore via basename (swap files, .DS_Store, etc); whereas the whole-path filter wants to be a whitelist, since we assume there's only a small well-structured set of places we want hakyll to explore (templates directory, posts directory, etc). Sure, we can get them to work together via boolean negation, but it's triggering my design-smell sense...

[1]: This automatic discovery wouldn't help the user explicitly say what's out of bounds (e.g., so she can catch typos in her rules); but it would address the performance side of the problem. Not that I've ever experienced performance problems, even when hakyll was traversing all sorts of things it shouldn't ;)

[2]: The thing I was trying to do was a linting/debugging pass to statically detect when two rules overlap either at the source, or at the destination. That's quite a bit trickier since it involves Routes which inexplicably hide IO under the hood. (Yes, I know that's because of metadataRoute; but since that's the only built-in route that isn't pure, it seems a bit like overkill.) Whereas, since Pattern doesn't hide IO under the hood, maybe doing this automatic detection would end up being feasible?

In the meantime, note that you can place all files required for compilation of the website into a directory (e.g. web-src), and use this as the provider directory (which is '.' by default).

Ooh, how on earth did I miss that? I'd been planning on manually implementing a hackish version of that in the filesystem, but that makes it much easier :)