fix(client): make our credential helper the only one the repo consults
Live QA against the beta forge failed its first push with 'Failed to authenticate user' while the config held a valid token. Cause: credential.helper is a list accumulated across system/global/repo config, and this machine has osxkeychain (Xcode gitconfig) plus store (~/.gitconfig). A stale entry for the forge host answered before our helper. The same list is a token leak in the other direction: git calls approve on every helper after a successful auth, so 'store' writes the forge token into ~/.git-credentials in plaintext -- undoing the 0600 config and the no-token-in-URL rule. Confirmed accidentally during QA when a verification clone with a URL-embedded token re-created exactly that entry. Fix: set an empty credential.helper first (git reads that as 'forget the inherited list'), then add ours -- in install_credential_helper and in the git clone inside get. 2 regression tests, one of which drives 'git credential fill' against a poisoned outer helper. 143 tests.
This commit is contained in:
@@ -116,6 +116,31 @@ restore point into a *new* directory and refuses a non-empty destination.
|
|||||||
Someone restoring a backup is already having a bad day; overwriting the files
|
Someone restoring a backup is already having a bad day; overwriting the files
|
||||||
they still have would make the recovery tool the second disaster.
|
they still have would make the recovery tool the second disaster.
|
||||||
|
|
||||||
|
## The credential helper must be the ONLY helper (found by live QA)
|
||||||
|
|
||||||
|
`credential.helper` is a list that accumulates across system, global and repo
|
||||||
|
config, and git asks every helper in it. A stock mac already has two —
|
||||||
|
`osxkeychain` from Xcode's gitconfig, and `store` from many people's
|
||||||
|
`~/.gitconfig` — and they lose in both directions:
|
||||||
|
|
||||||
|
* **reading:** a stale entry for the forge host answers before our helper, so
|
||||||
|
pushes fail `remote: Failed to authenticate user` long after the token was
|
||||||
|
rotated, and nothing in this tool's config explains why. This is exactly
|
||||||
|
how the first live-QA run failed;
|
||||||
|
* **writing:** git calls `approve` on every helper after a successful auth,
|
||||||
|
so `store` copies the forge token into `~/.git-credentials` **in
|
||||||
|
plaintext**. Keeping the token in a 0600 file and out of remote URLs buys
|
||||||
|
nothing if git then hands it to a plaintext store.
|
||||||
|
|
||||||
|
So `install_credential_helper` (and the `git clone` in `get`) sets an **empty**
|
||||||
|
`credential.helper` first, which resets the inherited list, then adds ours.
|
||||||
|
Exactly one helper serves this repo.
|
||||||
|
|
||||||
|
Corollary worth remembering: a token embedded in a remote URL gets saved by
|
||||||
|
`store` on first use. During QA a verification clone with a URL-embedded
|
||||||
|
token re-created the very entry that had just been cleaned out. That is the
|
||||||
|
whole reason this client passes tokens through a helper and never a URL.
|
||||||
|
|
||||||
## Device identity
|
## Device identity
|
||||||
|
|
||||||
`link` mints a uuid on first run and persists it in `~/.granthi-sync/config.json`
|
`link` mints a uuid on first run and persists it in `~/.granthi-sync/config.json`
|
||||||
@@ -356,7 +381,7 @@ deleted it again, `DELETE …/tokens/{id}` returning 204 under basic auth):
|
|||||||
|
|
||||||
## Tests
|
## Tests
|
||||||
|
|
||||||
* `python3 -m unittest discover -s tests` — 141 tests. The v1.2 additions
|
* `python3 -m unittest discover -s tests` — 143 tests. The v1.2 additions
|
||||||
cover: a snapshot capturing uncommitted work while HEAD, the index and the
|
cover: a snapshot capturing uncommitted work while HEAD, the index and the
|
||||||
working tree stay byte-identical; snapshots landing outside `refs/heads`;
|
working tree stay byte-identical; snapshots landing outside `refs/heads`;
|
||||||
an unchanged tree not being re-pushed; a diverged folder still being backed
|
an unchanged tree not being re-pushed; a diverged folder still being backed
|
||||||
@@ -368,7 +393,9 @@ deleted it again, `DELETE …/tokens/{id}` returning 204 under basic auth):
|
|||||||
disk; mode detection; `list` filtering; `get --all` skipping what is
|
disk; mode detection; `list` filtering; `get --all` skipping what is
|
||||||
already present, defaulting to snapshot mode, and shouting about
|
already present, defaulting to snapshot mode, and shouting about
|
||||||
truncation; `restore` writing a new folder, refusing a non-empty
|
truncation; `restore` writing a new folder, refusing a non-empty
|
||||||
destination, and leaving the working tree alone.
|
destination, and leaving the working tree alone; and the credential helper
|
||||||
|
being the only one the repo consults, proven by driving
|
||||||
|
`git credential fill` against a deliberately poisoned outer helper.
|
||||||
* Earlier suite: autocommit/ff/diverged
|
* Earlier suite: autocommit/ff/diverged
|
||||||
logic against real temp git repos (including "diverged never touches the
|
logic against real temp git repos (including "diverged never touches the
|
||||||
remote"), config 0600 handling (including umask-proof creation and a
|
remote"), config 0600 handling (including umask-proof creation and a
|
||||||
|
|||||||
@@ -544,7 +544,27 @@ def credential_helper_value():
|
|||||||
|
|
||||||
|
|
||||||
def install_credential_helper(folder):
|
def install_credential_helper(folder):
|
||||||
git(folder, "config", "credential.helper", credential_helper_value())
|
"""Make OUR helper the only one this repo consults.
|
||||||
|
|
||||||
|
credential.helper is a LIST that accumulates across system, global and
|
||||||
|
repo config, and git asks every helper in order. On a stock mac there are
|
||||||
|
already two (`osxkeychain` from Xcode's gitconfig, `store` from many
|
||||||
|
people's ~/.gitconfig), and they lose both ways:
|
||||||
|
|
||||||
|
* reading -- a stale entry for the forge host answers first, so pushes
|
||||||
|
fail with "Failed to authenticate user" long after the token was
|
||||||
|
rotated, and nothing in this tool's config explains why;
|
||||||
|
* writing -- git calls `approve` on every helper after a successful
|
||||||
|
auth, so `store` copies the forge token into ~/.git-credentials in
|
||||||
|
PLAINTEXT. Keeping the token in a 0600 file and out of remote URLs
|
||||||
|
is pointless if git hands it to a plaintext store on first use.
|
||||||
|
|
||||||
|
An empty value resets the inherited list, so replace-all-then-add leaves
|
||||||
|
exactly one helper: this script.
|
||||||
|
"""
|
||||||
|
git(folder, "config", "--replace-all", "credential.helper", "")
|
||||||
|
git(folder, "config", "--add", "credential.helper",
|
||||||
|
credential_helper_value())
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------
|
# --------------------------------------------------------------------------
|
||||||
@@ -756,7 +776,12 @@ def clone_one(cfg, full_name, dest, mode=None):
|
|||||||
# persists it into the new repo's config. --origin names the remote
|
# persists it into the new repo's config. --origin names the remote
|
||||||
# 'granthi' up front so `watch` picks the folder up without a rename.
|
# 'granthi' up front so `watch` picks the folder up without a rename.
|
||||||
proc = subprocess.run(
|
proc = subprocess.run(
|
||||||
|
# The empty -c resets the inherited helper list (see
|
||||||
|
# install_credential_helper) so a stale keychain/store entry cannot
|
||||||
|
# answer for the forge host during the clone, and the token cannot
|
||||||
|
# leak into a plaintext store afterwards.
|
||||||
["git", "clone",
|
["git", "clone",
|
||||||
|
"-c", "credential.helper=",
|
||||||
"-c", f"credential.helper={credential_helper_value()}",
|
"-c", f"credential.helper={credential_helper_value()}",
|
||||||
"--origin", "granthi", clone_url, dest],
|
"--origin", "granthi", clone_url, dest],
|
||||||
capture_output=True, text=True)
|
capture_output=True, text=True)
|
||||||
|
|||||||
@@ -858,5 +858,47 @@ class TestMarkerRoundTrip(GitScenarioBase):
|
|||||||
self.assertEqual(client.read_marker(self.local), {})
|
self.assertEqual(client.read_marker(self.local), {})
|
||||||
|
|
||||||
|
|
||||||
|
class TestCredentialHelperIsolation(GitScenarioBase):
|
||||||
|
"""A repo-local helper is not enough on a normal machine: git consults
|
||||||
|
system + global helpers too, and they both shadow us and copy the token
|
||||||
|
into plaintext. Found by live QA against the beta forge, not by a unit
|
||||||
|
test -- so it gets one now."""
|
||||||
|
|
||||||
|
def test_install_leaves_exactly_one_helper(self):
|
||||||
|
run_git(self.local, "config", "--add", "credential.helper", "store")
|
||||||
|
client.install_credential_helper(self.local)
|
||||||
|
# --get-all merges system + global + local, so entries inherited from
|
||||||
|
# the machine still appear. What matters is that the last two are the
|
||||||
|
# reset and ours: git reads an empty value as "forget every helper
|
||||||
|
# inherited so far", so nothing before it can answer.
|
||||||
|
helpers = run_git(self.local, "config", "--get-all",
|
||||||
|
"credential.helper").splitlines()
|
||||||
|
self.assertEqual(helpers[-2], "", helpers)
|
||||||
|
self.assertIn("git-credential", helpers[-1])
|
||||||
|
# the repo-level 'store' this test added is gone, not merely outvoted
|
||||||
|
self.assertNotIn("store", helpers)
|
||||||
|
|
||||||
|
def test_inherited_helper_cannot_answer_for_the_forge(self):
|
||||||
|
"""The end-to-end property: with a poisoned outer helper configured,
|
||||||
|
the credential git actually resolves is ours."""
|
||||||
|
fake = os.path.join(self.tmp, "poison.sh")
|
||||||
|
with open(fake, "w") as f:
|
||||||
|
f.write("#!/bin/sh\n"
|
||||||
|
"echo username=wrong-user\necho password=stale-token\n")
|
||||||
|
os.chmod(fake, 0o755)
|
||||||
|
run_git(self.local, "config", "--add", "credential.helper",
|
||||||
|
f"!{shlex.quote(fake)}")
|
||||||
|
client.save_config({"gitea_base": "http://forge.example:3041",
|
||||||
|
"login": "alice", "token": "the-right-token"})
|
||||||
|
client.install_credential_helper(self.local)
|
||||||
|
out = subprocess.run(
|
||||||
|
["git", "-C", self.local, "credential", "fill"],
|
||||||
|
input="protocol=http\nhost=forge.example:3041\n\n",
|
||||||
|
capture_output=True, text=True, env=dict(
|
||||||
|
GIT_ENV, GRANTHI_SYNC_HOME=os.environ["GRANTHI_SYNC_HOME"]))
|
||||||
|
self.assertIn("password=the-right-token", out.stdout)
|
||||||
|
self.assertNotIn("stale-token", out.stdout)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user