From 24462f460ee22b368c333522099c54e0e3d2ec9f Mon Sep 17 00:00:00 2001 From: fred Date: Fri, 21 Aug 2026 14:42:36 +0000 Subject: [PATCH] docs(git-wrappers): correct the stale --login fallback and document the test suite's identity split (#1354) --- packages/mosaic/framework/tools/git/README.md | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/mosaic/framework/tools/git/README.md b/packages/mosaic/framework/tools/git/README.md index 377c913c..caf14e01 100644 --- a/packages/mosaic/framework/tools/git/README.md +++ b/packages/mosaic/framework/tools/git/README.md @@ -30,7 +30,9 @@ The Gitea API token is **never passed on a curl command line.** An `Authorizatio ### `--login` override -Both `pr-review.sh` and `issue-comment.sh` accept an optional `--login ` flag that overrides the automatically detected Gitea login for that single invocation. The override selects **which credential the REST write, the `/user` identity lookup, and the read-back all use** — its token is resolved from the tea config for that login name (`get_gitea_token_for_login`), falling back to the repo host's credential when no login is named. The resolved login is **host- and port-bound**: the login's configured URL host **and effective port** (the scheme's default port — 80 for `http`, 443 for `https` — applies when a port is omitted, symmetrically on both sides) must match the repo remote's, so a login name shared across hosts (or an override configured for a different Gitea, including one on a different port of the same host) can never send one host's credential to another — a host or port mismatch fails closed rather than leaking a cross-host token. Resolving the acting identity and the read-back from the _same_ login that performs the write is essential: a write performed under an overridden login must be verified against that login's identity, not the host default's. Callers who need a different login than the host default should pass `--login `. +Both `pr-review.sh` and `issue-comment.sh` accept an optional `--login ` flag that overrides the automatically detected Gitea login for that single invocation. The override selects **which credential the REST write, the `/user` identity lookup, and the read-back all use** — its token is resolved from the tea config for that login name (`get_gitea_token_for_login`). + +**With no `--login`, there is no tea lookup at all.** The acting credential is the calling identity's own, resolved by `get_gitea_token` (see "Per-agent Gitea identity" below), and there is deliberately no fallback from it. These wrappers previously _guessed_ a login from the repo host and looked that guess up in the tea config; on a shared-account host the guess resolved to the shared login, so an unqualified call authored its write as that account rather than as the caller. Since `get_gitea_token_for_login` matches by login **name** and performs no authentication check, a dead shared credential still resolved at rc=0 and the identity-aware resolver was never reached. A caller passing no `--login` is asking to act as itself, so `--login` is now the only route to the tea store (#1351). The resolved login is **host- and port-bound**: the login's configured URL host **and effective port** (the scheme's default port — 80 for `http`, 443 for `https` — applies when a port is omitted, symmetrically on both sides) must match the repo remote's, so a login name shared across hosts (or an override configured for a different Gitea, including one on a different port of the same host) can never send one host's credential to another — a host or port mismatch fails closed rather than leaking a cross-host token. Resolving the acting identity and the read-back from the _same_ login that performs the write is essential: a write performed under an overridden login must be verified against that login's identity, not the host default's. Callers who need a different login than the host default should pass `--login `. As a durable successor to this mechanism, consider giving each reviewer/approver slot its own dedicated Gitea login credential, so that author≠reviewer holds at the credential level rather than relying on wrapper-level `--login` bookkeeping. This is a recommendation for future hardening, not something implemented by this flag. @@ -125,6 +127,32 @@ otherwise careful never to touch. Because identity is already resolved per-workt (`mosaic.gitIdentity`), the correct granularity for registering the helper is per-clone too, so a documented manual step is the right shape here, not a global auto-write. +### Running these tests + +`MOSAIC_GIT_IDENTITY` is inherited into each test's sandbox `HOME`, and **the tests disagree +about which value they need**, so no single ambient value passes all 29. Measured on `next` at +`a480ee83`, two full passes differing only in that variable: + +| tests | identity exported | identity unset | +| ----------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- | -------------- | +| `gitea-login-resolution`, `issue-comment-readback`, `issue-create-interactive-auth`, `pr-edit`, `pr-merge-gitea-empty-uid`, `pr-metadata-gitea` | **fail** | pass | +| `issue-close-fail-closed` | pass | **fail** | +| remaining 22 | pass | pass | + +The six fail because inside a sandbox `HOME` the identity has no `fleet/agents//` +directory, so it is classified as a **service identity**, its store is unpopulated, and the +resolver correctly refuses with `Refusing to borrow another slot's token`. That is the +documented fail-closed behaviour above, reached from a state the test never intended. +`issue-close-fail-closed` is the mirror image: it asserts that no comment POST is attempted, so +it needs an identity resolving to an empty slot, and with the variable unset the shared account +answers and the POST goes through. + +These read as wrapper regressions rather than as environment. Two seats independently +misdiagnosed them as a patch defect while reviewing #1352. Until each test controls its own +value (#1353), `env -u MOSAIC_GIT_IDENTITY` is the closest thing to a clean run at 28/29, with +`issue-close-fail-closed` the expected failure — and **"the suite passes" is not a statement +anyone can make here without naming the ambient value that produced it.** + ### PowerShell parity `detect-platform.ps1`'s Gitea wrappers authenticate through `tea` CLI logins