docs(git-wrappers): correct the stale --login fallback, document the test suite's identity split
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
Two corrections to packages/mosaic/framework/tools/git/README.md.
1. The "--login override" section still described the guessed-login fallback that
#1352 removed: "falling back to the repo host's credential when no login is
named". That is no longer what happens and it describes the defect as though
it were the design. With no --login there is now no tea lookup at all; the
acting credential is the calling identity's own via get_gitea_token, with no
fallback. #1352 should have carried this edit and did not.
2. New "Running these tests" section. MOSAIC_GIT_IDENTITY is inherited into each
test's sandbox HOME and the tests disagree about which value they need, so no
ambient value passes all 29. Measured on next at a480ee83 across two full
passes differing only in that variable: six fail with it exported, one fails
with it unset, and the failing sets are disjoint. The six hit the documented
fail-closed refusal from a state they never intended; issue-close-fail-closed
asserts the opposite and needs the identity set. Both directions read as
wrapper regressions rather than as environment, and two seats independently
misdiagnosed them as a patch defect while reviewing #1352.
The durable fix is for each test to control its own value; filed as #1353. This
documents the split so a red run is diagnosable in the meantime, and states
plainly that "the suite passes" is not a claim anyone can make here without
naming the ambient value behind it.
Formatting verified with the lockfile-pinned [email protected] (no install in this
worktree, so npx with the pin named). Control: a deliberately malformed file
returns rc=1 from the same command.
This commit is contained in:
fred
@@ -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 <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.
|
||||
|
||||
@@ -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/<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
|
||||
|
||||
`detect-platform.ps1`'s Gitea wrappers authenticate through `tea` CLI logins
|
||||
|
||||
Reference in New Issue
Block a user