git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
To: git@vger.kernel.org
Cc: dirk@ed4u.de, sunshine@sunshineco.com, peff@peff.net,
	gitster@pobox.com, jrnieder@gmail.com,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Subject: [RFC PATCH v6 0/2] credential-store: prevent fatal errors
Date: Wed, 29 Apr 2020 13:35:44 -0700	[thread overview]
Message-ID: <20200429203546.56753-1-carenas@gmail.com> (raw)
In-Reply-To: <20200429003303.93583-1-carenas@gmail.com>

with the added checks for invalid URLs in credentials, any locally
modified store files which might have empty lines or even comments
were reported[1] as failing to parse as valid credentials.

document better the format for the credential file and make sure
any failures are still handled gently by the new code to avoid
regressions but make sure that any invalid credentials are flagged
as such instead of being silently ignored.

ideally would like to add a test against the use of cert:// to make
sure that is still well supported but don't know of any use case or
if it is being used with store (AFAIK, it could and at least in theory
it should still work fine with the added validations), hence sending
this as an RFC.

also, there is no real reason to have 2 patches but I am keeping them
separated because the first one should be AFAIK otherwise almost ready.

but considering that we are validating since v4 against a missing
protocol and printing a warning since v5, it seems we should then
fully avoid all those silently ignored issues as well (as shown in
patch 2) which I hope we could squash and release together (if agreed)

[1] https://stackoverflow.com/a/61420852/5005936

Carlo Marcelo Arenas Belón (2):
  credential-store: warn instead of fatal for bogus lines from store
  credential-store: warn for any incomplete credentials instead of using

---
v6:
* get rid of redacter and only use line number for context
* make validation more strict to also catch incomplete credentials
v5:
* q_to_tab this round, with a single echo to make sure empty line
  is covered, as that seems to be a popular issue
* rebase on top of jc/credential-store-file-format-doc
* implement a redacter for credentials to use on errors to avoid
  leaking passwordss
v4:
* use credential_from_url_gently instead as shown by Jonathan
* add documentation to clarify "comments" is not a supported feature
v3:
* avoid using q_to_cr as suggested by Peff
* a more verbose commit message and slightly more complete documentation
v2:
* use a here-doc for clarity as suggested by Eric
* improve commit message and include documentation

 Documentation/git-credential-store.txt | 12 +++-
 credential-store.c                     | 23 +++++---
 t/t0302-credential-store.sh            | 80 ++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 10 deletions(-)


base-commit: 272281efcc18fcedd248597b8859f343cae1c5a0
-- 
2.26.2.569.g1d74ac4d14


  parent reply	other threads:[~2020-04-29 20:36 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-26 23:47 [PATCH] git-credential-store: skip empty lines and comments from store Carlo Marcelo Arenas Belón
2020-04-27  0:19 ` Eric Sunshine
2020-04-27  0:46   ` Carlo Marcelo Arenas Belón
2020-04-27  8:42 ` [PATCH v2] " Carlo Marcelo Arenas Belón
2020-04-27 11:52   ` Jeff King
2020-04-27 12:25     ` Carlo Marcelo Arenas Belón
2020-04-27 14:43       ` Eric Sunshine
2020-04-27 17:47     ` Junio C Hamano
2020-04-27 19:09       ` Jeff King
2020-04-27 12:59   ` [PATCH v3] " Carlo Marcelo Arenas Belón
2020-04-27 13:48     ` Philip Oakley
2020-04-28  1:49       ` Carlo Marcelo Arenas Belón
2020-04-29 10:09         ` Philip Oakley
2020-04-27 15:39     ` Dirk
2020-04-27 18:09     ` Junio C Hamano
2020-04-27 19:18       ` Jeff King
2020-04-27 20:43         ` Junio C Hamano
2020-04-27 21:10           ` Jeff King
2020-04-28  1:37             ` Carlo Marcelo Arenas Belón
2020-04-27 23:49           ` Carlo Marcelo Arenas Belón
2020-04-28  5:25           ` Jonathan Nieder
2020-04-28  5:41             ` Jeff King
2020-04-28  7:18               ` Carlo Marcelo Arenas Belón
2020-04-28  8:16                 ` Jeff King
2020-04-28 11:25                   ` Carlo Marcelo Arenas Belón
2020-04-28 10:58             ` Stefan Tauner
2020-04-28 16:03             ` Junio C Hamano
2020-04-28 21:14               ` Carlo Marcelo Arenas Belón
2020-04-28 21:17                 ` Junio C Hamano
2020-04-28 10:48     ` [PATCH v4 0/4] credential-store: prevent fatal errors Carlo Marcelo Arenas Belón
2020-04-28 10:52       ` [PATCH v4 1/4] credential-store: document the file format a bit more Carlo Marcelo Arenas Belón
2020-04-28 10:52         ` [PATCH v4 2/4] git-credential-store: skip empty lines and comments from store Carlo Marcelo Arenas Belón
2020-04-28 16:09           ` Eric Sunshine
2020-04-28 16:42             ` Carlo Marcelo Arenas Belón
2020-04-28 10:52         ` [PATCH v4 3/4] git-credential-store: fix (WIP) Carlo Marcelo Arenas Belón
2020-04-28 16:11           ` Eric Sunshine
2020-04-28 17:14             ` Carlo Marcelo Arenas Belón
2020-04-28 10:52         ` [PATCH v4 4/4] credential-store: make sure there is no regression with missing scheme Carlo Marcelo Arenas Belón
2020-04-28 16:06         ` [PATCH v4 1/4] credential-store: document the file format a bit more Eric Sunshine
2020-04-28 18:18           ` Junio C Hamano
2020-04-28 18:15         ` Junio C Hamano
2020-04-29  0:33       ` [PATCH v5] credential-store: warn instead of fatal for bogus lines from store Carlo Marcelo Arenas Belón
2020-04-29  4:36         ` Junio C Hamano
2020-04-29  7:31           ` Carlo Marcelo Arenas Belón
2020-04-29 16:46             ` Junio C Hamano
2020-04-29 20:35         ` Carlo Marcelo Arenas Belón [this message]
2020-04-29 20:35           ` [RFC PATCH v6 1/2] " Carlo Marcelo Arenas Belón
2020-04-29 21:05             ` Junio C Hamano
2020-04-29 21:17               ` Junio C Hamano
2020-04-29 20:35           ` [RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of using Carlo Marcelo Arenas Belón
2020-04-29 21:12             ` Junio C Hamano
2020-04-29 21:49               ` [RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of usingy Carlo Marcelo Arenas Belón
2020-04-29 22:04                 ` Junio C Hamano
2020-04-29 23:23           ` [PATCH v6] credential-store: warn instead of fatal for bogus lines from store Carlo Marcelo Arenas Belón
2020-04-29 23:47             ` Junio C Hamano
2020-04-29 23:57               ` Junio C Hamano
2020-04-30  1:00               ` Carlo Marcelo Arenas Belón
2020-04-30  1:19             ` [PATCH v7] " Carlo Marcelo Arenas Belón
2020-04-30  9:29               ` [PATCH v8] " Carlo Marcelo Arenas Belón
2020-04-30 16:06               ` [PATCH v9] " Carlo Marcelo Arenas Belón
2020-04-30 20:21                 ` Junio C Hamano
2020-04-30 21:14                   ` Junio C Hamano
2020-05-01  0:30                   ` Carlo Marcelo Arenas Belón
2020-05-01  1:40                     ` Junio C Hamano
2020-05-01  2:24                       ` Carlo Arenas
2020-05-01  5:27                         ` Junio C Hamano
2020-05-01 13:57                           ` Carlo Marcelo Arenas Belón
2020-05-01 18:59                             ` Junio C Hamano
2020-05-01  3:21                 ` [RFC PATCH v10] credential-store: warn/ignore for bogus lines from store file Carlo Marcelo Arenas Belón
2020-05-01  5:18                   ` [RFC PATCH v10 2/1] credential-store: warn also for store and erase Carlo Marcelo Arenas Belón
2020-05-01  5:35                     ` Junio C Hamano
2020-05-02 18:16                 ` [PATCH v10] credential-store: ignore bogus lines from store file Carlo Marcelo Arenas Belón
2020-05-02 20:47                   ` Junio C Hamano
2020-05-02 21:23                     ` Carlo Marcelo Arenas Belón
2020-05-02 21:53                     ` Carlo Marcelo Arenas Belón
2020-05-03  0:44                       ` Junio C Hamano
2020-05-03 10:06                     ` Jeff King
2020-05-02 21:05                   ` Carlo Marcelo Arenas Belón
2020-05-02 22:34                   ` [PATCH v11] " Carlo Marcelo Arenas Belón

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429203546.56753-1-carenas@gmail.com \
    --to=carenas@gmail.com \
    --cc=dirk@ed4u.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).