aboutsummaryrefslogtreecommitdiff
path: root/doc/bugs/removal_of_transient_pages.mdwn
blob: 8048ad9cce9accf7fddfdb14e30ce3385dc06a18 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
The remove plugin cannot remove [[todo/transient_pages]].

> this turns out to be harder than
> I'd hoped, because I don't want to introduce a vulnerability in the
> non-regular-file detection, so I'd rather defer that. --[[smcv]]

This is particularly a problem for tag pages, and autoindex
created pages. So both plugins default to not creating transient
pages, until this is fixed.  --[[Joey]] 

> I'll try to work out which of the checks are required for security
> and which are just nice-to-have, but I'd appreciate any pointers
> you could give. --[[smcv]]

>> I assume by "non-regular file", you are referring to the check
>> in remove that the file "Must exist on disk, and be a regular file" ?
>> --[[Joey]] 

>>> Yes. It's not entirely clear to me why that's there... --s

>>>> Yeah, 2461ce0de6231bfeea4d98c86806cdbb85683297 doesn't really
>>>> say, and I tend to assume that when I've written paranoid code
>>>> it's there for a reason. I think that here the concern was that
>>>> the file might be in some underlay that the user should not be able
>>>> to affect by web edits. The `-f` check seems rather redundant,
>>>> surely if it's in `%pagesources` ikiwiki has already verified it's
>>>> safe. --[[Joey]] 

----

[[!template id=gitbranch branch=smcv/ready/transient-rm author="[[Simon McVittie|smcv]]"]]

Here's a branch. It special-cases the `$transientdir`, but in such a way
that the special case could easily be extended to other locations where
deletion should be allowed.

It also changes `IkiWiki::prune()` to optionally stop pruning empty
parent directories at the point where you'd expect it to (for instance,
previously it would remove the `$transientdir` itself, if it turns out
to be empty), and updates callers.

The new `prune` API looks like this:

    IkiWiki::prune("$config{srcdir}/$file", $config{srcdir});

with the second argument optional. I wonder whether it ought to look
more like `writefile`:

    IkiWiki::prune($config{srcdir}, $file);

although that would be either an incompatible change to internal API
(forcing all callers to update to 2-argument), or being a bit
inconsistent between the one-and two-argument forms. Thoughts?

--[[smcv]]