git.oblomov.eu Git - ikiwiki/blob - doc/bugs/removal_of_transient_pages.mdwn

   1 The remove plugin cannot remove [[todo/transient_pages]].
   2
   3 > this turns out to be harder than
   4 > I'd hoped, because I don't want to introduce a vulnerability in the
   5 > non-regular-file detection, so I'd rather defer that. --[[smcv]]
   6
   7 This is particularly a problem for tag pages, and autoindex
   8 created pages. So both plugins default to not creating transient
   9 pages, until this is fixed.  --[[Joey]]
  10
  11 > I'll try to work out which of the checks are required for security
  12 > and which are just nice-to-have, but I'd appreciate any pointers
  13 > you could give. --[[smcv]]
  14
  15 >> I assume by "non-regular file", you are referring to the check
  16 >> in remove that the file "Must exist on disk, and be a regular file" ?
  17 >> --[[Joey]]
  18
  19 >>> Yes. It's not entirely clear to me why that's there... --s
  20
  21 >>>> Yeah, 2461ce0de6231bfeea4d98c86806cdbb85683297 doesn't really
  22 >>>> say, and I tend to assume that when I've written paranoid code
  23 >>>> it's there for a reason. I think that here the concern was that
  24 >>>> the file might be in some underlay that the user should not be able
  25 >>>> to affect by web edits. The `-f` check seems rather redundant,
  26 >>>> surely if it's in `%pagesources` ikiwiki has already verified it's
  27 >>>> safe. --[[Joey]]
  28
  29 ----
  30
  31 [[!template id=gitbranch branch=smcv/ready/transient-rm author="[[Simon McVittie|smcv]]"]]
  32
  33 Here's a branch. It special-cases the `$transientdir`, but in such a way
  34 that the special case could easily be extended to other locations where
  35 deletion should be allowed.
  36
  37 It also changes `IkiWiki::prune()` to optionally stop pruning empty
  38 parent directories at the point where you'd expect it to (for instance,
  39 previously it would remove the `$transientdir` itself, if it turns out
  40 to be empty), and updates callers.
  41
  42 The new `prune` API looks like this:
  43
  44     IkiWiki::prune("$config{srcdir}/$file", $config{srcdir});
  45
  46 with the second argument optional. I wonder whether it ought to look
  47 more like `writefile`:
  48
  49     IkiWiki::prune($config{srcdir}, $file);
  50
  51 although that would be either an incompatible change to internal API
  52 (forcing all callers to update to 2-argument), or being a bit
  53 inconsistent between the one-and two-argument forms. Thoughts?
  54
  55 --[[smcv]]
  56
  57 > I've applied the branch as-is, so this bug is [[done]].
  58 > `prune` is not an exported API so changing it would be ok..
  59 > I think required 2-argument would be better, but have not checked
  60 > all the call sites to see if the `$file` is available split out
  61 > as that would need. --[[Joey]]
  62
  63 [[!template id=gitbranch branch=smcv/ready/prune author="[[Simon McVittie|smcv]]"]]
  64
  65 >> Try this, then? I had to make some changes to `attachment`
  66 >> to make the split versions available. I suggest reviewing
  67 >> patch-by-patch.
  68 >>
  69 >> I also tried to fix a related bug which I found while testing it:
  70 >> the special case for renaming held attachments didn't seem to work.
  71 >> (`smcv/wip/rename-held`.) Unfortunately, it seems that with that
  72 >> change, the held attachment is committed to the `srcdir` when you
  73 >> rename it, which doesn't seem to be the intention either? --[[smcv]]