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. > 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... > "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. > + # 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.) Martin
