Thread (222 messages) 222 messages, 11 authors, 2020-04-06

Re: [PATCH v5 09/15] bugreport: generate config safelist based on docs

From: Martin Ågren <hidden>
Date: 2020-01-30 22:34:38

On Fri, 24 Jan 2020 at 04:35, [off-list ref] 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@gmail.com/ (local)
quoted hunk ↗ jump to hunk
--- 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help