[PATCH b4 v2 0/3] review: add support for fetching patchwork token via git credentials

[PATCH b4 v2 0/3] review: add support for fetching patchwork token via git credentials

[Brian Masney]

Currently the patchwork token needs to be stored plaintext in your git
configuration. This series adds support for fetching the token via git
credentials, which allows storing this externally in a password manager
if desired.

Signed-off-by: Brian Masney <masneyb@kernel.org>
---
Changes in v2:
- Changed to use git credentials to fetch the patchwork key.
- Link to v1: https://patch.msgid.link/20261015-pw-key-command-v1-0-6daa9b8cee2a@kernel.org

---
Brian Masney (3):
      review: centralize lookup of patchwork key in one place
      review: add support for fetching patchwork key via git credentials
      review: tests: add tests for the patchwork key

 docs/config.rst                    |  29 ++++++++++
 src/b4/__init__.py                 |  56 +++++++++++++++++-
 src/b4/review/_review.py           |   4 +-
 src/b4/review_tui/_common.py       |   2 +-
 src/b4/review_tui/_entry.py        |   2 +-
 src/b4/review_tui/_tracking_app.py |   2 +-
 src/tests/test___init__.py         | 114 +++++++++++++++++++++++++++++++++++++
 7 files changed, 202 insertions(+), 7 deletions(-)
---
base-commit: 6be7c6139d7734d84afddf989fa2874935b6d85b
change-id: 20261015-pw-key-command-12c3634b2e8f

Best regards,
--  
Brian Masney <masneyb@kernel.org>

[PATCH b4 v2 1/3] review: centralize lookup of patchwork key in one place

[Brian Masney]

Introduce get_pw_key() so that the patchwork key lookup is done in one
place. This is in preparation for adding a pw-key-command configuration
option.

Assisted-by: LLM
Signed-off-by: Brian Masney <masneyb@kernel.org>
---
 src/b4/__init__.py                 | 14 ++++++++++++--
 src/b4/review/_review.py           |  4 ++--
 src/b4/review_tui/_common.py       |  2 +-
 src/b4/review_tui/_entry.py        |  2 +-
 src/b4/review_tui/_tracking_app.py |  2 +-
 5 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index eb04a2af55ee..16b7208f0b09 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -2504,7 +2504,7 @@ class LoreMessage:
     @staticmethod
     def get_patchwork_data_by_msgid(msgid: str) -> Dict[str, str]:
         config = get_main_config()
-        pwkey = str(config.get('pw-key', ''))
+        pwkey = get_pw_key(config)
         pwurl = str(config.get('pw-url', ''))
         pwproj = str(config.get('pw-project', ''))
         if not (pwkey and pwurl and pwproj):
@@ -6402,6 +6402,16 @@ def get_smtp(
     return SMTPConnector(_connect), fromaddr
 
 
+def get_pw_key(config: Optional[ConfigDictT] = None) -> str:
+    """Return the Patchwork API key from b4.pw-key.
+
+    Returns an empty string when no key is configured.
+    """
+    if config is None:
+        config = get_main_config()
+    return str(config.get('pw-key', '') or '')
+
+
 def get_patchwork_session(pwkey: str, pwurl: str) -> Tuple[requests.Session, str]:
     session = requests.session()
     session.headers.update(
@@ -6418,7 +6428,7 @@ def get_patchwork_session(pwkey: str, pwurl: str) -> Tuple[requests.Session, str
 def patchwork_set_state(msgids: List[str], state: str) -> None:
     # Do we have a pw-key defined in config?
     config = get_main_config()
-    pwkey = str(config.get('pw-key', ''))
+    pwkey = get_pw_key(config)
     pwurl = str(config.get('pw-url', ''))
     pwproj = str(config.get('pw-project', ''))
     if not (pwkey and pwurl and pwproj):
diff --git a/src/b4/review/_review.py b/src/b4/review/_review.py
index b69a7f094bc7..c5e4289ea930 100644
--- a/src/b4/review/_review.py
+++ b/src/b4/review/_review.py
@@ -4232,7 +4232,7 @@ def pw_update_series_state(
     Patchwork is not configured), False on failure.
     """
     config = b4.get_main_config()
-    pwkey = str(config.get('pw-key', ''))
+    pwkey = b4.get_pw_key(config)
     pwurl = str(config.get('pw-url', ''))
     if not (pwkey and pwurl):
         # Patchwork not configured, nothing to do
@@ -4270,7 +4270,7 @@ def pw_lookup_series_id(msgid: str) -> Optional[int]:
     configured or does not know the message.
     """
     config = b4.get_main_config()
-    pwkey = str(config.get('pw-key', ''))
+    pwkey = b4.get_pw_key(config)
     pwurl = str(config.get('pw-url', ''))
     pwproj = str(config.get('pw-project', ''))
     if not (pwkey and pwurl and pwproj):
diff --git a/src/b4/review_tui/_common.py b/src/b4/review_tui/_common.py
index 43db8c9358bf..205762956517 100644
--- a/src/b4/review_tui/_common.py
+++ b/src/b4/review_tui/_common.py
@@ -361,7 +361,7 @@ class CheckRunnerMixin:
 
         # Patchwork config for _builtin_patchwork
         config = b4.get_main_config()
-        pwkey = str(config.get('pw-key', ''))
+        pwkey = b4.get_pw_key(config)
         pwurl = str(config.get('pw-url', ''))
 
         # Dump tracking data to a temp file for external scripts.  This is
diff --git a/src/b4/review_tui/_entry.py b/src/b4/review_tui/_entry.py
index 700c5eae0e6a..275d6703949f 100644
--- a/src/b4/review_tui/_entry.py
+++ b/src/b4/review_tui/_entry.py
@@ -151,7 +151,7 @@ def run_tracking_tui(
             if branch_name == TrackingApp.PATCHWORK_SENTINEL:
                 # User pressed [p]atchwork — run PwApp and loop back
                 config = b4.get_main_config()
-                pwkey = str(config.get('pw-key', ''))
+                pwkey = b4.get_pw_key(config)
                 pwurl = str(config.get('pw-url', ''))
                 pwproj = str(config.get('pw-project', ''))
                 if pwkey and pwurl and pwproj:
diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index 4ae826ee5b9e..75d652149e29 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -1159,7 +1159,7 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
         self._db_mtime: float = 0.0
         # Detect patchwork configuration
         config = b4.get_main_config()
-        self._pwkey = str(config.get('pw-key', ''))
+        self._pwkey = b4.get_pw_key(config)
         self._pwurl = str(config.get('pw-url', ''))
         self._pwproj = str(config.get('pw-project', ''))
         # Remember last snooze choices within the session

-- 
2.56.0

[PATCH b4 v2 2/3] review: add support for fetching patchwork key via git credentials

[Brian Masney]

Introduce a b4.pw-user and support for fetching the patchwork key via
git credentials.

Assisted-by: LLM
Signed-off-by: Brian Masney <masneyb@kernel.org>
---
 docs/config.rst    | 29 +++++++++++++++++++++++++++++
 src/b4/__init__.py | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/docs/config.rst b/docs/config.rst
index 593129e8ae70..7087883c0a22 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -1022,6 +1022,35 @@ integrate your b4 workflow with patchwork.
 
      Default: ``None``
 
+   :term:`b4.pw-user`
+     Your username on the patchwork server. When this is set and
+     :term:`b4.pw-key` is not, b4 asks your git credential helpers for the
+     password of this user on the :term:`b4.pw-url` host, and uses it as
+     the API key. This lets you keep the key in a password manager instead
+     of storing it in plain text in your git config. b4 asks at most once
+     per invocation. For example::
+
+         pw-url = https://patchwork.kernel.org/
+         pw-user = jdoe
+
+     To store the key with a credential helper that supports it::
+
+         printf 'protocol=https\nhost=patchwork.kernel.org\nusername=jdoe\npassword=%s\n' \
+             "$PW_API_KEY" | git credential approve
+
+     You can also inject the key via a password manager by placing the following
+     in your .gitconfig:
+
+        [credential "https://user@patchwork.kernel.org"]
+        helper = "!f() { echo password=$(pass korg-patchwork-token); }; f"
+
+     If no helper returns a password, git may prompt for one on the
+     terminal. This setting is ignored in the in-tree ``.b4-config`` file.
+
+     Default: ``None``
+
+     .. versionadded:: v0.17
+
 .. _contributor_settings:
 
 Contributor-oriented settings
diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index 16b7208f0b09..045e5f6e7b08 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -392,6 +392,8 @@ LORENODE: Optional[liblore.LoreNode] = None
 URL_SERVERS: Dict[str, Tuple[str, bool]] = dict()
 # Indicates that we've cleaned cache already
 _CACHE_CLEANED = False
+# Patchwork API keys returned by git credential helpers, keyed by (url, user)
+_PW_KEY_CACHE: Dict[Tuple[str, str], str] = dict()
 # DNS answers for the life of the process (see _setup_dns_resolver)
 _DNS_CACHE = dns.resolver.LRUCache()
 # Used to track send-email alias replacements
@@ -5000,6 +5002,11 @@ def _setup_main_config(
         'trailer*',
         'pw-*',
     ]
+    # Never take these from an in-tree .b4-config: they are personal and
+    # they make b4 ask git credential helpers for a secret.
+    wtdeny = [
+        'pw-user',
+    ]
     multivals = [
         'keyringsrc',
         'am-perpatch-check-cmd',
@@ -5015,6 +5022,9 @@ def _setup_main_config(
             wtconfig = get_config_from_git(r'b4\..*', multivals=multivals, source=wtcfg)
             logger.debug('wtcfg=%s', wtconfig)
             for key, val in wtconfig.items():
+                if key in wtdeny:
+                    logger.debug('wtcfg: ignoring %s', key)
+                    continue
                 if key in multivals:
                     val = [_val_to_path(topdir, x) for x in val]
                 else:
@@ -6403,13 +6413,45 @@ def get_smtp(
 
 
 def get_pw_key(config: Optional[ConfigDictT] = None) -> str:
-    """Return the Patchwork API key from b4.pw-key.
+    """Return the Patchwork API key.
 
-    Returns an empty string when no key is configured.
+    Use b4.pw-key when it is set.  Otherwise, if b4.pw-user is set, ask the
+    git credential helpers for the password of that user on the b4.pw-url
+    host, so the key can live in a password manager instead of the git
+    config.  The result is cached for the life of the process, so the
+    helpers are asked at most once.  Returns an empty string when no key is
+    configured or the lookup fails.
     """
     if config is None:
         config = get_main_config()
-    return str(config.get('pw-key', '') or '')
+    pwkey = str(config.get('pw-key', '') or '')
+    pwuser = str(config.get('pw-user', '') or '')
+    pwurl = str(config.get('pw-url', '') or '')
+    if pwkey or not (pwuser and pwurl):
+        return pwkey
+
+    cachekey = (pwurl, pwuser)
+    if cachekey in _PW_KEY_CACHE:
+        return _PW_KEY_CACHE[cachekey]
+
+    parsed = urllib.parse.urlparse(pwurl)
+    if parsed.scheme and parsed.netloc:
+        logger.debug('Asking git credential helpers for the patchwork key')
+        gc_pass = git_credential_fill(
+            None, protocol=parsed.scheme, host=parsed.netloc, username=pwuser
+        )
+        pwkey = gc_pass or ''
+        if not pwkey:
+            logger.critical(
+                'No patchwork key for %s@%s from git credential helpers',
+                pwuser,
+                parsed.netloc,
+            )
+    else:
+        logger.critical('Could not parse pw-url: %s', pwurl)
+
+    _PW_KEY_CACHE[cachekey] = pwkey
+    return pwkey
 
 
 def get_patchwork_session(pwkey: str, pwurl: str) -> Tuple[requests.Session, str]:

-- 
2.56.0

[PATCH b4 v2 3/3] review: tests: add tests for the patchwork key

[Brian Masney]

Add tests for the various ways that the patchwork key can be provided.

Assisted-by: LLM
Signed-off-by: Brian Masney <masneyb@kernel.org>
---
 src/tests/test___init__.py | 114 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/src/tests/test___init__.py b/src/tests/test___init__.py
index 764829d7e756..5d5c9dd3836b 100644
--- a/src/tests/test___init__.py
+++ b/src/tests/test___init__.py
@@ -2924,3 +2924,117 @@ class TestGetAmMessageFromLines:
         lines = payload.decode().splitlines()
         assert 'From the manual, bar() must be called here.' in lines
         assert '>From an old thread.' in lines
+
+
+class TestGetPwKey:
+    """b4.pw-user fetches the Patchwork key from git credential helpers."""
+
+    @pytest.fixture(autouse=True)
+    def _clear_cache(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        monkeypatch.setattr(b4, '_PW_KEY_CACHE', {})
+        # Never fall back to prompting on the terminal
+        monkeypatch.setenv('GIT_TERMINAL_PROMPT', '0')
+        monkeypatch.delenv('GIT_ASKPASS', raising=False)
+        monkeypatch.delenv('SSH_ASKPASS', raising=False)
+
+    @staticmethod
+    def _set_helper(
+        monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path, body: str
+    ) -> pathlib.Path:
+        """Replace all credential helpers with a script that runs body."""
+        helper = tmp_path / 'helper.sh'
+        helper.write_text(f'#!/bin/sh\necho "$@" >> {tmp_path}/calls\n{body}\n')
+        helper.chmod(0o755)
+        count = int(os.environ.get('GIT_CONFIG_COUNT', '0'))
+        # An empty value resets any helpers from the user's own config
+        for i, val in enumerate(('', f'!{helper}')):
+            monkeypatch.setenv(f'GIT_CONFIG_KEY_{count + i}', 'credential.helper')
+            monkeypatch.setenv(f'GIT_CONFIG_VALUE_{count + i}', val)
+        monkeypatch.setenv('GIT_CONFIG_COUNT', str(count + 2))
+        return tmp_path / 'calls'
+
+    def test_plain_key(self) -> None:
+        assert b4.get_pw_key({'pw-key': 'secret'}) == 'secret'
+
+    def test_unset(self) -> None:
+        assert b4.get_pw_key({}) == ''
+
+    def test_no_user(
+        self, monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path
+    ) -> None:
+        """Without pw-user, credential helpers are never asked."""
+        calls = self._set_helper(monkeypatch, tmp_path, 'echo password=nope')
+        assert b4.get_pw_key({'pw-url': 'https://pw.example.org/'}) == ''
+        assert not calls.exists()
+
+    def test_key_wins(
+        self, monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path
+    ) -> None:
+        calls = self._set_helper(monkeypatch, tmp_path, 'echo password=nope')
+        config: b4.ConfigDictT = {
+            'pw-key': 'secret',
+            'pw-user': 'me',
+            'pw-url': 'https://pw.example.org/',
+        }
+        assert b4.get_pw_key(config) == 'secret'
+        assert not calls.exists()
+
+    def test_credential_helper(
+        self, monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path
+    ) -> None:
+        req = tmp_path / 'request'
+        self._set_helper(
+            monkeypatch, tmp_path, f'cat > {req}\necho password=fromhelper'
+        )
+        config: b4.ConfigDictT = {
+            'pw-user': 'me',
+            'pw-url': 'https://pw.example.org:8443/',
+        }
+        assert b4.get_pw_key(config) == 'fromhelper'
+        request = req.read_text().splitlines()
+        assert 'protocol=https' in request
+        assert 'host=pw.example.org:8443' in request
+        assert 'username=me' in request
+
+    def test_credential_helper_runs_once(
+        self, monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path
+    ) -> None:
+        calls = self._set_helper(monkeypatch, tmp_path, 'echo password=key')
+        config: b4.ConfigDictT = {
+            'pw-user': 'me',
+            'pw-url': 'https://pw.example.org/',
+        }
+        assert b4.get_pw_key(config) == 'key'
+        assert b4.get_pw_key(config) == 'key'
+        assert calls.read_text().count('get') == 1
+
+    def test_credential_helper_empty(
+        self,
+        monkeypatch: pytest.MonkeyPatch,
+        tmp_path: pathlib.Path,
+        caplog: pytest.LogCaptureFixture,
+    ) -> None:
+        self._set_helper(monkeypatch, tmp_path, 'true')
+        config: b4.ConfigDictT = {
+            'pw-user': 'me',
+            'pw-url': 'https://pw.example.org/',
+        }
+        with caplog.at_level(logging.CRITICAL, logger='b4'):
+            assert b4.get_pw_key(config) == ''
+        assert 'No patchwork key for me@pw.example.org' in caplog.text
+
+    def test_bad_url(self, caplog: pytest.LogCaptureFixture) -> None:
+        config: b4.ConfigDictT = {'pw-user': 'me', 'pw-url': 'pw.example.org'}
+        with caplog.at_level(logging.CRITICAL, logger='b4'):
+            assert b4.get_pw_key(config) == ''
+        assert 'Could not parse pw-url' in caplog.text
+
+    def test_ignored_in_worktree_config(self, gitdir: str) -> None:
+        """A cloned tree's .b4-config must not get to ask for credentials."""
+        wtcfg = pathlib.Path(gitdir) / '.b4-config'
+        wtcfg.write_text(
+            '[b4]\n\tpw-user = stolen\n\tpw-url = https://pw.example.org\n'
+        )
+        b4._setup_main_config(topdir=gitdir)
+        assert b4.MAIN_CONFIG.get('pw-url') == 'https://pw.example.org'
+        assert b4.MAIN_CONFIG.get('pw-user') is None

-- 
2.56.0