On Thu, Jan 30, 2020 at 11:34:24PM +0100, Martin Ågren wrote: > On Fri, 24 Jan 2020 at 04:35, <emilyshaffer@xxxxxxxxxx> wrote: > > Add a new step to the build to generate a safelist of git-config > > variables which are appropriate to include in the output of > > git-bugreport. New variables can be added to the safelist by annotating > > their documentation in Documentation/config with the > > "bugreport" macro, which is recognized by AsciiDoc and > > AsciiDoctor. > > > > Some configs are private in nature, and can contain remote URLs, > > passwords, or other sensitive information. In the event that a user > > doesn't notice their information while reviewing a bugreport, that user > > may leak their credentials to other individuals, mailing lists, or bug > > tracking tools inadvertently. Heuristic blocklisting of configuration > > keys is imperfect and prone to false negatives; given the nature of the > > information which can be leaked, a safelist is more reliable. > > I sort of wonder whether safelist/blocklist is to prefer over > whitelist/blacklist, or if it's the other way round. The former are new > to me, whereas the latter are the terms I would have used. But that's > just me, of course. I was a little surprised, that's all. Eh. I think the following things are true: - Whitelist/blacklist has a "smell" of discrimination, whether that's the true etymology or not. - Those with experience in the field can easily understand what whitelist or blacklist means. - Safelist/blocklist do not "smell" the same way. - It is easy to tell what "safelist" means: "a list of stuff which is safe." No experience needed. So, while it's new, I think it's not harmful. I see only a no-op or positive impact from using this term instead of whitelist/blacklist. Computer science seems to have quite a few terms which fall into this long-standing but potentially negative area, so I don't mind looking for alternatives where it's harmless to do so. > > > Implement a new no-op "bugreport" macro for use as > > "bugreport:include[x]" to annotate the config keys that should be > > included in the automatically generated safelist. Use "exclude" for the > > others. > > > > With Asciidoctor, it's ok to say "bugreport:include[]", but AsciiDoc > > seems to want something between the brackets. A bit unfortunate, but > > not a huge problem -- we'll just provide an "x". > > I recognize this reasoning :-) and I'm not terribly opposed to it, but > after some nights' sleeping on this, I have to wonder if > "annotate:bugreport[include]" wouldn't be better than "bugreport[x]" > with that ugly "x". Maybe this isn't the biggest problem, but if we > expect this macro to eventually sit right next to ~90% of all our config > variables... Hm. I wanted to say, "Ok, but I don't know how to do that, so can you help?" But I think that's all the more reason that I should do it ;) Ok. I will try and change it to annotate:bugreport[include] like you suggested, and hopefully learn more about asciidoc macros :) > > > "doc-diff" reports that this macro doesn't render at all. That is, > > these are both empty after this commit: > > > > cd Documentation > > ./doc-diff --asciidoctor :/"bugreport: add tool" HEAD > > ./doc-diff --asciidoc :/"bugreport: add tool" HEAD > > That was true in [1], but alas, no more. In that patch, it's sort of > obvious from the diff how it adds a "class" which "end"s. > > [1] https://lore.kernel.org/git/20190817203846.31609-1-martin.agren@xxxxxxxxx/ > > > --- a/Documentation/asciidoctor-extensions.rb > > +++ b/Documentation/asciidoctor-extensions.rb > > @@ -37,6 +37,10 @@ module Git > > output = output.sub(/<\/refmeta>/, new_tags + "</refmeta>") > > end > > output > > + > > + class BugReportProcessor < Asciidoctor::Extensions::InlineMacroProcessor > > + def process(parent, action, attrs) > > + "" > > end > > end > > end > > But this one doesn't add an "end", and Asciidoctor trips up badly. Ok, I'll have a look. I'm sure I copied something badly. > > > + # The bugreport macro does nothing as far as rendering is > > + # concerned -- we just grep for it in the sources. > > + inline_macro Git::Documentation::BugReportProcessor, :bugreport > > (I never much liked this copy-paste comment then, and I still don't, to > be honest.) I'll see if I can find a reasonable alternative (or remove it). - Emily
