On Thu, Jan 30, 2020 at 11:34:24PM +0100, Martin Ågren wrote:

On Fri, 24 Jan 2020 at 04:35, [off-list ref] wrote:

quoted

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.

quoted

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 :)

quoted

"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@gmail.com/ (local)

quoted

--- 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.

quoted

+  # 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