docs(git-wrappers): correct the stale --login fallback and document the test suite's identity split (#1354)
ci/woodpecker/push/publish Pipeline was successful

This commit was merged in pull request #1354.
This commit is contained in:
2026-08-21 14:42:36 +00:00
parent a480ee83dc
commit 24462f460e
+29 -1
View File
@@ -30,7 +30,9 @@ The Gitea API token is **never passed on a curl command line.** An `Authorizatio
### `--login` override ### `--login` override
Both `pr-review.sh` and `issue-comment.sh` accept an optional `--login <name>` 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 <reviewer-login>`. Both `pr-review.sh` and `issue-comment.sh` accept an optional `--login <name>` 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 <reviewer-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. 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 (`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. 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/<id>/`
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 ### PowerShell parity
`detect-platform.ps1`'s Gitea wrappers authenticate through `tea` CLI logins `detect-platform.ps1`'s Gitea wrappers authenticate through `tea` CLI logins