Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
835d6705d6 | ||
|
|
7eb57acdbe | ||
|
|
ea14038355 | ||
|
|
1b6682eee5 | ||
|
|
062f8d1863 |
@@ -104,6 +104,10 @@ FORGE_MAX_PAGES = 40 # 2000 repos; a guard against an unbounded paging loop
|
||||
# Under refs/heads a machine taking a snapshot every 30s would bury the
|
||||
# user's real branches.
|
||||
BACKUP_NS = "refs/granthi-backup"
|
||||
_SNAPSHOT_IDENT = {"GIT_AUTHOR_NAME": "granthi-sync",
|
||||
"GIT_AUTHOR_EMAIL": "[email protected]",
|
||||
"GIT_COMMITTER_NAME": "granthi-sync",
|
||||
"GIT_COMMITTER_EMAIL": "[email protected]"}
|
||||
SNAPSHOT_TS_FMT = "%Y%m%dT%H%M%SZ"
|
||||
|
||||
# Retention. Unbounded snapshots are a disk leak with no way to use them, so
|
||||
@@ -301,15 +305,33 @@ def build_snapshot(folder):
|
||||
args = ["commit-tree", tree, "-m", f"granthi snapshot: {ts}"]
|
||||
if head:
|
||||
args += ["-p", head]
|
||||
# The worktree tree alone loses STAGED-ONLY work. Stage a hunk, edit the
|
||||
# file further, lose the laptop, and the snapshot holds only the later
|
||||
# worktree version -- the carefully staged one is gone. git itself keeps
|
||||
# index and worktree as separate states, so the backup must too.
|
||||
# The user's real index is read WITHOUT touching it, and when it differs
|
||||
# from both HEAD and the worktree it rides along as a second parent, so
|
||||
# it is reachable from the snapshot. (codex review, P2.)
|
||||
rc_idx, index_tree = git(folder, "write-tree", check=False)
|
||||
index_tree = index_tree.strip()
|
||||
if rc_idx == 0 and index_tree and index_tree != tree:
|
||||
head_tree_now = ""
|
||||
if head:
|
||||
head_tree_now = git(folder, "rev-parse", f"{head}^{{tree}}")[1].strip()
|
||||
if index_tree != head_tree_now:
|
||||
icommit_args = ["commit-tree", index_tree, "-m",
|
||||
f"granthi snapshot (staged): {ts}"]
|
||||
if head:
|
||||
icommit_args += ["-p", head]
|
||||
rc_ic, icommit = git(folder, *icommit_args, check=False,
|
||||
env=_SNAPSHOT_IDENT)
|
||||
if rc_ic == 0 and icommit.strip():
|
||||
args += ["-p", icommit.strip()]
|
||||
# Snapshots are parented on HEAD and nothing else -- deliberately NOT
|
||||
# chained to the previous snapshot. Chaining would keep every old
|
||||
# snapshot reachable from the newest one, so pruning a ref would free
|
||||
# nothing and retention would be decorative.
|
||||
_, commit = git(folder, *args,
|
||||
env={"GIT_AUTHOR_NAME": "granthi-sync",
|
||||
"GIT_AUTHOR_EMAIL": "[email protected]",
|
||||
"GIT_COMMITTER_NAME": "granthi-sync",
|
||||
"GIT_COMMITTER_EMAIL": "[email protected]"})
|
||||
_, commit = git(folder, *args, env=_SNAPSHOT_IDENT)
|
||||
return commit.strip(), tree
|
||||
finally:
|
||||
if os.path.exists(tmp_index):
|
||||
@@ -838,7 +860,7 @@ def clone_one(cfg, full_name, dest, mode=None):
|
||||
return meta
|
||||
|
||||
|
||||
def resolve_granted(cfg, want, repos=None):
|
||||
def resolve_granted(cfg, want, repos=None, truncated=False):
|
||||
"""Turn what the user typed into the repo the forge actually grants them.
|
||||
|
||||
`get notes` used to mean `<your-login>/notes` and nothing else. On a real
|
||||
@@ -857,7 +879,7 @@ def resolve_granted(cfg, want, repos=None):
|
||||
return parse_repo_arg(want, cfg["login"])
|
||||
if repos is None:
|
||||
try:
|
||||
repos, _truncated = list_repos(cfg)
|
||||
repos, truncated = list_repos(cfg)
|
||||
except (SystemExit, ValueError, OSError) as e:
|
||||
# Best effort. If the listing is unreachable, a name the user
|
||||
# typed in full must still clone, and a bare name should degrade
|
||||
@@ -876,8 +898,18 @@ def resolve_granted(cfg, want, repos=None):
|
||||
f"{want!r} is ambiguous — {len(matches)} repos you can see have "
|
||||
f"that name:\n{listed}\nRe-run with the owner, e.g. "
|
||||
f"granthi-sync get {sorted(matches)[0]}")
|
||||
# Nothing granted by that name. Fall back to the caller's own namespace so
|
||||
# the message names a concrete repo instead of a guess.
|
||||
# Nothing granted by that name. If the listing was TRUNCATED the answer is
|
||||
# unknown rather than absent -- falling back to <login>/<name> could clone a
|
||||
# different repo that happens to exist under your own account. Refuse and
|
||||
# ask for the owner. (codex review, P3.)
|
||||
if truncated:
|
||||
raise SystemExit(
|
||||
f"could not confirm {want!r}: your repo list was truncated at "
|
||||
f"{FORGE_MAX_PAGES * FORGE_PAGE_LIMIT} repos, so a match may exist "
|
||||
f"beyond it. Re-run with the owner, e.g. "
|
||||
f"granthi-sync get <owner>/{want}")
|
||||
# Otherwise the listing was complete and simply has no such repo; name a
|
||||
# concrete one so the clone error is precise.
|
||||
return parse_repo_arg(want, cfg["login"])
|
||||
|
||||
|
||||
@@ -1341,13 +1373,14 @@ def cmd_bootstrap(args):
|
||||
into = os.path.abspath(args.into or ".")
|
||||
print(f"workspace: {name}\n")
|
||||
|
||||
granted_repos, _trunc = list_repos(cfg)
|
||||
granted_repos, granted_truncated = list_repos(cfg)
|
||||
granted = {r.get("full_name") for r in granted_repos}
|
||||
pulled = skipped = denied = 0
|
||||
for repo in ws["repos"]:
|
||||
want = repo["name"]
|
||||
try:
|
||||
full = resolve_granted(cfg, want, repos=granted_repos)
|
||||
full = resolve_granted(cfg, want, repos=granted_repos,
|
||||
truncated=granted_truncated)
|
||||
except SystemExit as e:
|
||||
log(f"AMBIGUOUS {want}: {e}")
|
||||
denied += 1
|
||||
|
||||
+48
-1
@@ -80,6 +80,9 @@ TEST_MODE_ENV = "GRANTHI_LINK_ALLOW_TEST_MODE"
|
||||
|
||||
# Sentinel: create_user hit a 409 (someone else created the login first).
|
||||
USER_CREATE_CONFLICT = object()
|
||||
# Sentinel: the address already belongs to another forge account, which is a
|
||||
# 409 the caller can act on -- not a 502 that reads like the service is down.
|
||||
EMAIL_IN_USE = object()
|
||||
|
||||
LOGIN_SAFE = re.compile(r"[^a-zA-Z0-9._-]+")
|
||||
|
||||
@@ -292,6 +295,14 @@ def check_config_perms(path, euid=None):
|
||||
# Identity map: zitadel sub -> gitea login (JSON, 0600, atomic writes)
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
def _email_in_use_message(login, userinfo):
|
||||
email = userinfo.get("email") or "your address"
|
||||
return (f"cannot create the account '{login}': {email} already belongs to "
|
||||
f"a different account on this forge. If that other account is "
|
||||
f"yours, an operator must bind your identity to it; if it is not, "
|
||||
f"use a different address.")
|
||||
|
||||
|
||||
class IdentityStore:
|
||||
"""Persistent map of Zitadel `sub` -> Gitea login binding records.
|
||||
|
||||
@@ -404,6 +415,23 @@ class IdentityStore:
|
||||
self._write(data)
|
||||
return pending
|
||||
|
||||
def consume_invite(self, email, repo):
|
||||
"""Drop ONE applied grant, leaving any that failed still pending.
|
||||
|
||||
The whole-list `take_invites` is what made a transient forge error
|
||||
permanent; this removes only what actually landed.
|
||||
"""
|
||||
data = self._load()
|
||||
book = data.get("invites") or {}
|
||||
key = (email or "").strip().lower()
|
||||
entry = [e for e in book.get(key, []) if e.get("repo") != repo]
|
||||
if entry:
|
||||
book[key] = entry
|
||||
else:
|
||||
book.pop(key, None)
|
||||
data["invites"] = book
|
||||
self._write(data)
|
||||
|
||||
def peek_invites(self, email):
|
||||
book = self._load().get("invites") or {}
|
||||
return list(book.get((email or "").strip().lower(), []))
|
||||
@@ -660,6 +688,14 @@ class LinkService:
|
||||
headers=self._admin_hdr(), body=body)
|
||||
if status == 409:
|
||||
return USER_CREATE_CONFLICT
|
||||
if status == 422 and "e-mail already in use" in str(resp).lower():
|
||||
# The address belongs to a DIFFERENT forge account. Reported as its
|
||||
# own case because the generic path turns it into a bare 502, and
|
||||
# "502" sends the person looking for an outage when the real answer
|
||||
# is "that address is already somebody's account here". Hit live on
|
||||
# 2026-08-23: an operator moved an email onto another account and
|
||||
# the next sign-in failed with nothing but the number.
|
||||
return EMAIL_IN_USE
|
||||
if status != 201:
|
||||
return f"gitea admin user create failed (HTTP {status}): {resp}"
|
||||
return None
|
||||
@@ -716,6 +752,8 @@ class LinkService:
|
||||
"was not created by this service; refusing "
|
||||
"to re-create"}, None
|
||||
err = self.create_user(login, userinfo)
|
||||
if err is EMAIL_IN_USE:
|
||||
return 409, {"error": _email_in_use_message(login, userinfo)}, None
|
||||
if err and err is not USER_CREATE_CONFLICT:
|
||||
return 502, {"error": err}, None
|
||||
LOG.info("re-created service-managed gitea user %s", login)
|
||||
@@ -741,6 +779,8 @@ class LinkService:
|
||||
return 409, {"error": "login exists and is not linked "
|
||||
"to this identity"}, None
|
||||
LOG.info("user %s created concurrently; continuing", login)
|
||||
elif err is EMAIL_IN_USE:
|
||||
return 409, {"error": _email_in_use_message(login, userinfo)}, None
|
||||
elif err:
|
||||
return 502, {"error": err}, None
|
||||
else:
|
||||
@@ -1038,8 +1078,13 @@ class LinkService:
|
||||
# the only thing tying the promise to this person.
|
||||
return []
|
||||
with self.state.lock:
|
||||
pending = self.state.take_invites(email)
|
||||
pending = self.state.peek_invites(email)
|
||||
applied = []
|
||||
# PEEK, not take. Consuming the invite first means a transient 502 from
|
||||
# the forge destroys it: the person links successfully, gets no access,
|
||||
# and re-linking never retries because the promise is gone. Only the
|
||||
# grants that actually landed are removed, so a failure is retried on
|
||||
# the next link instead of being silently lost. (codex review, P2.)
|
||||
for grant in pending:
|
||||
status, resp = http_json(
|
||||
"PUT",
|
||||
@@ -1050,6 +1095,8 @@ class LinkService:
|
||||
body={"permission": grant.get("permission", "write")})
|
||||
if status in (200, 204):
|
||||
applied.append(grant["repo"])
|
||||
with self.state.lock:
|
||||
self.state.consume_invite(email, grant["repo"])
|
||||
self.audit.write("invite.applied", login=login,
|
||||
repo=grant["repo"],
|
||||
permission=grant.get("permission"),
|
||||
|
||||
+60
-12
@@ -29,9 +29,11 @@ GIT_ENV = {
|
||||
}
|
||||
|
||||
|
||||
def run_git(cwd, *args):
|
||||
return subprocess.run(["git", "-C", cwd] + list(args), check=True,
|
||||
capture_output=True, text=True, env=GIT_ENV).stdout.strip()
|
||||
def run_git(cwd, *args, strip=True):
|
||||
stdout = subprocess.run(["git", "-C", cwd] + list(args), check=True,
|
||||
capture_output=True, text=True,
|
||||
env=GIT_ENV).stdout
|
||||
return stdout.strip() if strip else stdout
|
||||
|
||||
|
||||
|
||||
@@ -905,15 +907,13 @@ class TestCredentialHelperIsolation(GitScenarioBase):
|
||||
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
|
||||
# Inspect the repo-local list so this assertion is deterministic even
|
||||
# when the machine has no inherited helper. Keep the leading newline:
|
||||
# it represents the empty reset value, not disposable whitespace.
|
||||
helpers = run_git(self.local, "config", "--local", "--get-all",
|
||||
"credential.helper", strip=False).splitlines()
|
||||
self.assertEqual(helpers, ["", client.credential_helper_value()])
|
||||
# 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):
|
||||
@@ -1310,5 +1310,53 @@ class TestDeviceFlowOutputIsVisible(unittest.TestCase):
|
||||
self.assertTrue(all(seen[:2]),
|
||||
"the URL and code must be printed with flush=True")
|
||||
|
||||
|
||||
class TestCodexReviewFindings(GitScenarioBase):
|
||||
"""Regressions for the three issues codex found in today's merged work."""
|
||||
|
||||
def test_staged_only_work_survives_a_snapshot(self):
|
||||
"""[P2] Stage a hunk, edit further, lose the laptop: the staged version
|
||||
must still be recoverable, not just the later worktree one."""
|
||||
self.write(self.local, "a.txt", "committed")
|
||||
run_git(self.local, "add", "-A")
|
||||
run_git(self.local, "commit", "-m", "base")
|
||||
self.write(self.local, "a.txt", "THE CAREFULLY STAGED VERSION")
|
||||
run_git(self.local, "add", "a.txt") # staged
|
||||
self.write(self.local, "a.txt", "later scratch edit") # worktree moved on
|
||||
|
||||
commit, tree = client.build_snapshot(self.local)
|
||||
|
||||
# the worktree state is the snapshot's own tree
|
||||
self.assertEqual(run_git(self.local, "show", f"{commit}:a.txt"),
|
||||
"later scratch edit")
|
||||
# ...and the staged state is reachable through the extra parent
|
||||
parents = run_git(self.local, "log", "-1", "--format=%P", commit).split()
|
||||
staged = [p for p in parents
|
||||
if run_git(self.local, "show", f"{p}:a.txt")
|
||||
== "THE CAREFULLY STAGED VERSION"]
|
||||
self.assertTrue(staged, f"staged version unreachable from {parents}")
|
||||
|
||||
def test_a_snapshot_does_not_disturb_the_index(self):
|
||||
self.write(self.local, "a.txt", "one")
|
||||
run_git(self.local, "add", "-A")
|
||||
run_git(self.local, "commit", "-m", "base")
|
||||
self.write(self.local, "a.txt", "staged")
|
||||
run_git(self.local, "add", "a.txt")
|
||||
before = run_git(self.local, "status", "--porcelain")
|
||||
client.build_snapshot(self.local)
|
||||
self.assertEqual(run_git(self.local, "status", "--porcelain"), before)
|
||||
|
||||
def test_a_truncated_listing_refuses_instead_of_guessing(self):
|
||||
"""[P3] A name that is merely beyond the page cap must not resolve to a
|
||||
different repo that happens to exist under your own account."""
|
||||
cfg = {"login": "alice", "gitea_base": "http://forge.example", "token": "t"}
|
||||
with self.assertRaises(SystemExit) as e:
|
||||
client.resolve_granted(cfg, "notes", repos=[], truncated=True)
|
||||
self.assertIn("truncated", str(e.exception))
|
||||
# a complete listing still falls back, because absence is then real
|
||||
self.assertEqual(
|
||||
client.resolve_granted(cfg, "notes", repos=[], truncated=False),
|
||||
"alice/notes")
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -77,6 +77,10 @@ class StubUpstream(BaseHTTPRequestHandler):
|
||||
if self.path == "/api/v1/admin/users":
|
||||
if body["username"] in st["users"]:
|
||||
return self._json(409, {"message": "user already exists"})
|
||||
if body.get("email") in st["users"].values():
|
||||
# real Gitea: emails are unique across accounts
|
||||
return self._json(422, {"message":
|
||||
f"e-mail already in use [email: {body['email']}]"})
|
||||
st["users"][body["username"]] = body["email"]
|
||||
st["created"].append(body)
|
||||
return self._json(201, {"login": body["username"]})
|
||||
@@ -960,6 +964,75 @@ class TestInvites(ServiceTestBase):
|
||||
_, mine = self.svc.audit_read({"token": self.alice["token"]})
|
||||
self.assertIn("invite", [e["event"] for e in mine["events"]])
|
||||
|
||||
|
||||
class TestInviteSurvivesAFailedGrant(ServiceTestBase):
|
||||
"""[P2, codex] A transient forge error must not destroy the promise."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.enable_test_mode()
|
||||
self.alice = self.stub_link("s1", "alice", email="[email protected]",
|
||||
verified=True, device_id="dev-a")[1]
|
||||
StubUpstream.state["repo_owner"]["alice/notes"] = "alice"
|
||||
StubUpstream.state["repo_owner"]["alice/reports"] = "alice"
|
||||
|
||||
def test_a_failed_grant_leaves_the_invite_pending_for_next_time(self):
|
||||
self.svc.invite({"token": self.alice["token"], "email": "[email protected]",
|
||||
"repos": ["notes"]})
|
||||
# the forge loses the repo mid-flight -> the PUT 404s
|
||||
StubUpstream.state["repo_owner"].pop("alice/notes")
|
||||
status, resp = self.stub_link("s9", "carol", email="[email protected]",
|
||||
verified=True, device_id="dev-c")
|
||||
self.assertEqual(status, 200) # sign-in still succeeds
|
||||
self.assertIsNone(resp.get("granted_repos"))
|
||||
# the promise is STILL THERE rather than silently consumed
|
||||
self.assertEqual([g["repo"] for g in self.svc.state.peek_invites("[email protected]")],
|
||||
["alice/notes"])
|
||||
# and it lands on the next link, once the repo is back
|
||||
StubUpstream.state["repo_owner"]["alice/notes"] = "alice"
|
||||
status, resp = self.stub_link("s9", "carol", email="[email protected]",
|
||||
verified=True, device_id="dev-c2")
|
||||
self.assertEqual(resp.get("granted_repos"), ["alice/notes"])
|
||||
self.assertEqual(self.svc.state.peek_invites("[email protected]"), [])
|
||||
|
||||
def test_a_partial_failure_only_consumes_what_landed(self):
|
||||
self.svc.invite({"token": self.alice["token"], "email": "[email protected]",
|
||||
"repos": ["notes", "reports"]})
|
||||
StubUpstream.state["repo_owner"].pop("alice/reports") # one of two fails
|
||||
_, resp = self.stub_link("s10", "dan", email="[email protected]",
|
||||
verified=True, device_id="dev-d")
|
||||
self.assertEqual(resp.get("granted_repos"), ["alice/notes"])
|
||||
self.assertEqual([g["repo"] for g in self.svc.state.peek_invites("[email protected]")],
|
||||
["alice/reports"])
|
||||
|
||||
|
||||
class TestDuplicateEmailIsExplained(ServiceTestBase):
|
||||
"""A 502 sends someone looking for an outage. The real answer is that the
|
||||
address already belongs to another account here -- say so. (Hit live on
|
||||
2026-08-23 when an operator moved an email onto a different account.)"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.enable_test_mode()
|
||||
# an existing account already holds the address
|
||||
StubUpstream.state["users"]["existing"] = "[email protected]"
|
||||
|
||||
def test_it_is_a_409_that_names_the_problem(self):
|
||||
status, resp = self.stub_link("s-new", "brandnew", email="[email protected]",
|
||||
verified=True, device_id="dev-x")
|
||||
self.assertEqual(status, 409, resp)
|
||||
msg = resp["error"]
|
||||
self.assertIn("[email protected]", msg)
|
||||
self.assertIn("already belongs to a different account", msg)
|
||||
self.assertIn("brandnew", msg) # names the login it tried
|
||||
self.assertNotIn("502", msg)
|
||||
|
||||
def test_a_normal_create_is_unaffected(self):
|
||||
status, resp = self.stub_link("s-ok", "fresh", email="[email protected]",
|
||||
verified=True, device_id="dev-y")
|
||||
self.assertEqual(status, 200, resp)
|
||||
self.assertIn("token", resp)
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user