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

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Wed, Feb 05, 2020 at 04:41:02PM -0800, Emily Shaffer 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 "annotate" macro,
> which is a no-op in 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.
> 
> However, it's possible that in some situations, an organization may be
> less concerned with privacy of things like remote URLs and branch names,
> and more concerned with ease of diagnosis for their support staff. In
> those cases, it may make more sense for that organization to modify the
> code to use a blocklist. To that end, we should try to mark configs which
> are definitely safe, and configs which are definitely unsafe, and leave
> blank configs which are somewhere in between. To mark a config as safe,
> add "annotate:bugreport[include]" to the corresponding line in the
> config documentation; to mark it as unsafe, add
> "annotate:bugreport[exclude]" instead.
> 
> Generating bugreport-config-safelist.h at build time by grepping the
> documentation for this new macro helps us prevent staleness. The macro
> itself is a no-op and should not alter the appearance of the
> documentation in either AsciiDoc or AsciiDoctor, confirmable by running:
> 
>   cd Documentation
>   ./doc-diff --asciidoctor HEAD^ HEAD
>   ./doc-diff --asciidoc HEAD^ HEAD
> 
> Diffing the rendered HTML shows that only inline comments were added,
> which shouldn't be a problem.
> 
> Additionally, add annotations to the sendemail config documentation in
> order to demonstrate a proof of concept.
> 
> Helped-by: Martin Ågren <martin.agren@gmail.com>
> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  .gitignore                              |  1 +
>  Documentation/asciidoc.conf             |  9 ++++
>  Documentation/asciidoctor-extensions.rb |  5 +++
>  Documentation/config/sendemail.txt      | 56 ++++++++++++-------------
>  Makefile                                |  7 ++++
>  generate-bugreport-config-safelist.sh   | 17 ++++++++
>  6 files changed, 67 insertions(+), 28 deletions(-)
>  create mode 100755 generate-bugreport-config-safelist.sh
> 
> diff --git a/.gitignore b/.gitignore
> index d89bf9e11e..bd2f49b996 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -192,6 +192,7 @@
>  /gitweb/static/gitweb.min.*
>  /config-list.h
>  /command-list.h
> +/bugreport-config-safelist.h
>  *.tar.gz
>  *.dsc
>  *.deb
> diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf
> index 8fc4b67081..663e06481f 100644
> --- a/Documentation/asciidoc.conf
> +++ b/Documentation/asciidoc.conf
> @@ -6,9 +6,14 @@
>  #
>  # Show Git link as: <command>(<section>); if section is defined, else just show
>  # the command.
> +#
> +# The annotate macro does nothing as far as rendering is
> +# concerned -- we just grep for it in the sources to populate
> +# things like the bugreport safelist.
>  
>  [macros]
>  (?su)[\\]?(?P<name>linkgit):(?P<target>\S*?)\[(?P<attrlist>.*?)\]=
> +(?su)[\\]?(?P<name>annotate):(?P<target>\S*?)\[(?P<attrlist>.*?)\]=
>  
>  [attributes]
>  asterisk=&#42;
> @@ -28,6 +33,8 @@ ifdef::backend-docbook[]
>  {0#<citerefentry>}
>  {0#<refentrytitle>{target}</refentrytitle><manvolnum>{0}</manvolnum>}
>  {0#</citerefentry>}
> +[annotate-inlinemacro]
> +{0#}
>  endif::backend-docbook[]
>  
>  ifdef::backend-docbook[]
> @@ -94,4 +101,6 @@ ifdef::backend-xhtml11[]
>  git-relative-html-prefix=
>  [linkgit-inlinemacro]
>  <a href="{git-relative-html-prefix}{target}.html">{target}{0?({0})}</a>
> +[annotate-inlinemacro]
> +<!-- -->
>  endif::backend-xhtml11[]
> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> index d906a00803..382bd8f6f4 100644
> --- 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 AnnotateProcessor < Asciidoctor::Extensions::InlineMacroProcessor
> +      def process(parent, target, attrs)
> +        ""

This change breaks building the documentation with Asciidoctor v1.5.8:

  /home/travis/.rvm/rubies/ruby-2.5.3/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in `require': /home/travis/build/git/git/Documentation/asciidoctor-extensions.rb:41: class definition in method body (SyntaxError)
  
  ...xtensions::InlineMacroProcessor
  
  ...                               ^
  
  /home/travis/build/git/git/Documentation/asciidoctor-extensions.rb:53: syntax error, unexpected end-of-input, expecting keyword_end


  https://travis-ci.org/git/git/jobs/647093871#L1127

>        end
>      end
>    end
> @@ -45,4 +49,5 @@ end
>  Asciidoctor::Extensions.register do
>    inline_macro Git::Documentation::LinkGitProcessor, :linkgit
>    postprocessor Git::Documentation::DocumentPostProcessor
> +  inline_macro Git::Documentation::AnnotateProcessor, :annotate
>  end