fred
3d2b712355
ci/woodpecker/pr/ci Pipeline failed
Two changes to one rule: a credential is resolved from exactly one place,
and an identity that cannot be resolved is refused rather than substituted.
FAIL CLOSED. Both readers ended in an unconditional fall-through to the
shared Gitea account whenever an identity did not resolve. Every seat in a
fleet therefore pushed, opened PRs and filed reviews under one account, and
a record made that way cannot be traced to the agent that made it
afterwards. The fallback now applies only where there is no attribution to
lose: a host with no fleet. Where seats exist, an unresolvable request emits
nothing, exits nonzero, explains itself on stderr, and — in the git helper —
appends a record naming the identity, host, reason and cwd, and no token
value, to ${MOSAIC_CREDENTIAL_SPOOL:-~/.local/state/mosaic-credential-escalations}.
A host runs a fleet when <brain>/fleet/agents exists, which is the signal
packages/mosaic/src/fleet/brain-home.ts already uses to decide a brain is
active, resolved the same way (MOSAIC_BRAIN_HOME, else ~/.mosaic). This is
what keeps the change a no-op for an operator who has not provisioned
per-slot tokens: no fleet directory, shared account, unchanged. It is also
why there is no environment variable to restore the old behavior — one would
reintroduce the substitution being removed.
STORE SELECTION. Both readers hardcoded ~/.config/mosaic/secrets/gitea-tokens,
so a seat's own secrets/ slot was invisible to the framework: a seat could
hold a valid credential and still be served the shared account. The store is
now chosen by what the identity is. An identity with a directory under
<brain>/fleet/agents/ is a seat and is read only from
<brain>/fleet/agents/<id>/secrets/; any other identity is a service identity
and is read from the framework store. There is no precedence between them
and no fallback from one to the other, so a seat with an empty slot is
refused even when a same-named token sits in the framework store. Two copies
of one credential are drift rather than redundancy, and drift surfaces as
the stale copy returning 401, which reads as a revoked token and sends
whoever debugs it somewhere else.
detect-platform.sh is in scope alongside git-credential-mosaic because they
are the two readers of these tokens. Patching only the git helper would make
"one credential, one location" true for push and fetch and false for
pr-create.sh, issue-create.sh and pr-review.sh, which is the harder failure
to notice.
TESTS. The three assertions that pinned the shared-account fall-through are
now fail-closed assertions, and a refusal is checked four independent ways:
nonzero exit, empty stdout, a stderr diagnostic naming identity and host,
and no shared token value anywhere in the output. The exit code alone would
pass against a helper that emitted the credential and then failed. Added:
seat-slot resolution, the no-cross-store-fallback case with a control
proving the framework-store file it declines to read is readable, no-identity
on a fleet host, the fleet gate firing on the default ~/.mosaic and not only
on an injected MOSAIC_BRAIN_HOME, and a cross-host leak check. Both suites
were run against the pre-change code as a control and fail there on exactly
the shared-token emission.
shellcheck is not installed on the authoring host, so the rewritten helper
is unlinted locally and CI is the first lint of it.
139 lines
14 KiB
Markdown
139 lines
14 KiB
Markdown
# Git provider wrappers
|
|
|
|
These scripts provide host-aware GitHub and Gitea issue, pull-request, milestone, and CI operations.
|
|
|
|
## Durable review provenance
|
|
|
|
A successful provider write command—or a wrapper message based only on that command's exit code—is **not** durable review provenance. Review comments, approvals, and change requests count as durable provenance only after the wrapper reads the created provider record back and verifies that it was created by _this_ write.
|
|
|
|
**The write is a direct Gitea REST `POST` that returns the created record's id.** Neither wrapper writes through `tea` — tea 0.11.1 can silently no-op while exiting 0 and cannot emit the id of a record it creates, so its exit code is worthless as proof of a durable write (#865). Instead:
|
|
|
|
- Comments (`issue-comment.sh`, and the `comment` action of `pr-review.sh`) `POST /api/v1/repos/{owner}/{repo}/issues/{index}/comments`, requiring a `201` and parsing the created comment's `id` from the response body.
|
|
- Reviews (`approve` / `request-changes`) `POST /api/v1/repos/{owner}/{repo}/pulls/{index}/reviews` with the `event` (`APPROVED` / `REQUEST_CHANGES`), the review `body`, and `commit_id` pinned to the PR's current head, then parse the created review's `id`. The review body travels _in the review submit itself_ — there is no separate detached comment to reconcile (a Gitea `REQUEST_CHANGES` review requires a non-empty body, which the submit carries).
|
|
|
|
**Verification keys on that exact provider-returned id.** The wrapper then `GET`s that one record directly — `GET /issues/comments/{id}` or `GET /pulls/{n}/reviews/{id}` — and requires that its `id` equals the created id, its **author login equals the acting identity** (resolved via `GET /api/v1/user` for the token in use), and, for comments, its body exactly matches what was submitted **and its returned web URL belongs to this exact provider and repository** (the `issue_url` / `pull_request_url` origin — scheme, host, and effective port — and full path, i.e. deployment prefix + exact `owner/repo` + kind + number, must match; a suffix/`endsWith` test would accept a look-alike host or a decoy path prefix, so the whole normalized URL is compared). The `comment` action of `pr-review.sh` additionally requires the returned resource be a **pull request** (a populated `pull_request_url`); a bare `issue_url` is rejected, so if issue `#N` exists but PR `#N` does not, an issue comment cannot be reported as a verified PR comment. (`issue-comment.sh` legitimately keeps the broader issue-or-PR acceptance.) For reviews, its state matches the requested action, its reviewed `commit_id` equals the PR head, **and its persisted body equals the submitted body** — an exact, presence- and type-checked equality (a missing/`null` persisted body no longer counts as an empty match), because Gitea can finalize/reuse a pending review id whose stored content was authored elsewhere, so the body is bound too. The write, the `/user` identity lookup, and the read-back all use the **same** credential — the effective login's token, or the host credential when no login is named — so the write is verified against the identity that actually performed it.
|
|
|
|
**A review's pinned head is re-checked after verification (current-head TOCTOU).** The `commit_id` is pinned to the PR head read _before_ the submit; between that read and the read-back the branch could advance (a force-push or a new commit), leaving a verified review attached to a now-superseded commit while the live tip carries unreviewed code. After the exact-id read-back succeeds, the wrapper re-reads the live PR head (`GET …/pulls/{n}`) and requires it still equals the submitted SHA; if the head advanced it fails closed (non-zero, no success line) rather than reporting a review that no longer covers the PR's current commit.
|
|
|
|
**This closes the concurrency window rather than documenting it.** Because verification keys on the id the create returned, a no-op create yields no id and fails closed with no list-scan fallback, and a _concurrent_ record — even one written by the _same_ identity with an identical body/state — has a _different_ id and cannot be mistaken for this write. There is no residual same-identity window: the earlier boundary-and-author heuristic (accept any `id > pre-write-max` with a matching author) is replaced entirely by exact-id attribution.
|
|
|
|
**Exact-id read-back is the sole authority.** Verification is a direct `GET` of the one record the create returned; there is no follow-up list enumeration. An earlier redundant pass that re-listed the record's page (`?limit=&page=1,2,…`) was removed: server-capped page sizes and list-pagination quirks made it a false-failure source (a durable, exact-id-verified record could be missed by a non-exhaustive enumeration), and it added nothing over the authoritative exact-id `GET`.
|
|
|
|
## Credential handling
|
|
|
|
The Gitea API token is **never passed on a curl command line.** An `Authorization: token <value>` argument would be visible to any local process that can read the process table (`ps` / `/proc/<pid>/cmdline`) for the lifetime of the request. Instead, every authenticated curl call writes the header into a private, mode-`0600` config file under `$TMPDIR` and passes it with `curl --config <file>` (`gitea_write_auth_config`), so only the file _path_ — never the token — appears in argv. Each such file is unlinked on every exit path (success and failure) by the caller's `RETURN` trap.
|
|
|
|
## `tea` invocation notes (Gitea)
|
|
|
|
- tea v0.11.1 has **no `comment` subcommand under `tea pr` or `tea issue`** — the `tea pr comment` / `tea issue comment` forms don't error, they silently fall through to a no-op and still exit 0, producing a false-success write (#865). tea's write subcommands (`tea comment`, `tea pr approve`/`reject`) also cannot report the id of the record they create, so their exit code cannot prove a durable write. These wrappers therefore do **not** write reviews or comments through `tea` at all; they use direct Gitea REST `POST`s that return the created record's id (see "Durable review provenance" above). `tea` is consulted only to enumerate the login list for host→login resolution.
|
|
- Because the review body is carried in the `POST …/reviews` submit itself, there is no separate detached review comment, and the historical `tea pr approve`/`reject` trailing-positional-argument vs. nonexistent `--comment`/`-comment` flag hazard (#835) no longer applies to these wrappers — no review comment is ever passed to `tea`.
|
|
|
|
### `--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>`.
|
|
|
|
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.
|
|
|
|
## Per-agent Gitea identity (Gate-16 author≠reviewer)
|
|
|
|
By default, git push/fetch (via `git-credential-mosaic`) and the API wrappers above (via
|
|
`detect-platform.sh`'s `get_gitea_token`) all authenticate as the single shared Gitea
|
|
account/token configured through `tools/_lib/credentials.sh`. That means every agent in a
|
|
fleet commits, pushes, and opens PRs under one identity — with no cryptographic
|
|
separation between an author and a reviewer.
|
|
|
|
Both `git-credential-mosaic` and `get_gitea_token()` resolve an optional **per-agent
|
|
identity**:
|
|
|
|
1. `MOSAIC_GIT_IDENTITY` environment variable, or
|
|
2. `git config --get mosaic.gitIdentity` (set per-worktree; persists on disk across
|
|
non-persistent shells — `git config mosaic.gitIdentity <agent-id>`), or
|
|
3. (git-credential-mosaic only) the username git itself supplies for the credential
|
|
request.
|
|
|
|
### Which store a credential is read from
|
|
|
|
The store is chosen by what the identity **is**, not by which file happens to exist first:
|
|
|
|
| The identity | Its credential is read from |
|
|
| --- | --- |
|
|
| has a directory at `<brain>/fleet/agents/<id>/` — it is a **seat** | `<brain>/fleet/agents/<id>/secrets/gitea-{usc,mosaicstack}-<id>.token` |
|
|
| does not — it is a **service identity** | `~/.config/mosaic/secrets/gitea-tokens/gitea-{usc,mosaicstack}-<id>.token` |
|
|
|
|
`<brain>` is `MOSAIC_BRAIN_HOME` if set, else `~/.mosaic` — the same resolution
|
|
`packages/mosaic/src/fleet/brain-home.ts` performs.
|
|
|
|
**There is no precedence between the two stores and no fallback from one to the other.**
|
|
A seat whose slot is empty is refused even when a same-named token sits in the framework
|
|
store. One credential lives in exactly one location: a second copy is drift rather than
|
|
redundancy, and the way drift surfaces is a stale copy returning 401, which reads as a
|
|
revoked token and sends whoever debugs it to the wrong place.
|
|
|
|
### What happens when nothing resolves
|
|
|
|
| identity resolves | token in its store | host runs a fleet | result |
|
|
| --- | --- | --- | --- |
|
|
| yes | yes | — | that identity + token |
|
|
| yes | no | — | **fail closed** |
|
|
| no | — | yes | **fail closed** |
|
|
| no | — | no | shared account, unchanged |
|
|
|
|
A host "runs a fleet" when `<brain>/fleet/agents` exists — the same signal `brain-home.ts`
|
|
uses to decide a brain is active.
|
|
|
|
Failing closed means: nothing is emitted, the exit status is nonzero, a stderr diagnostic
|
|
names the identity, its source, the store it resolved to and the path that was expected,
|
|
and `git-credential-mosaic` additionally appends a record (identity, host, reason, cwd —
|
|
never a token value) to `${MOSAIC_CREDENTIAL_SPOOL:-~/.local/state/mosaic-credential-escalations}`.
|
|
The git operation fails; nothing is attributed to anyone.
|
|
|
|
The shared-account fallback that used to cover these two cases is why a PR could be
|
|
authored, commented and merged under an account whose owner did not open it — every seat
|
|
shared one identity, so the record could not be traced back afterwards. An
|
|
under-provisioned agent is refused rather than handed the most privileged account
|
|
available.
|
|
|
|
**On a host with no fleet, nothing changes**: no `fleet/agents` directory means the shared
|
|
account still answers, so this is a no-op for an operator who has not provisioned per-slot
|
|
tokens. On a host that does run a fleet, a human doing manual git work needs an identity
|
|
of their own — `MOSAIC_GIT_IDENTITY=<id>` with a provisioned slot. There is deliberately no
|
|
environment variable that restores the fallback; one would reintroduce exactly the
|
|
substitution this removes.
|
|
|
|
### Enabling it for a clone
|
|
|
|
The framework installer syncs `git-credential-mosaic` to
|
|
`~/.config/mosaic/tools/git/git-credential-mosaic` (executable) on every install/update,
|
|
but does **not** register it as git's credential helper automatically. Registration is a
|
|
one-time, explicit step:
|
|
|
|
```bash
|
|
# Per-repo (recommended — scopes the helper to this clone only):
|
|
git config credential.helper "$HOME/.config/mosaic/tools/git/git-credential-mosaic"
|
|
|
|
# Per-worktree identity pin (Gate-16 separation):
|
|
git config mosaic.gitIdentity <agent-id>
|
|
```
|
|
|
|
This is deliberately **not** auto-registered on install/update: `credential.helper` is
|
|
global, order-sensitive git config (`~/.gitconfig`) that can already hold an
|
|
operator-chosen credential manager (keychain, `store`, `manager-core`, …) for
|
|
repositories unrelated to Mosaic. Silently inserting an entry on every framework
|
|
install/upgrade risks reordering or shadowing that operator-owned surface across the
|
|
whole host — the same operator-owned config the installer's manifest system is
|
|
otherwise careful never to touch. Because identity is already resolved per-worktree
|
|
(`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.
|
|
|
|
### PowerShell parity
|
|
|
|
`detect-platform.ps1`'s Gitea wrappers authenticate through `tea` CLI logins
|
|
(`Get-GiteaLoginForHost`), not a raw-token `get_gitea_token`-equivalent function — there
|
|
is nothing to prepend the identity-resolution block to on the PowerShell side. A native
|
|
PowerShell git-credential helper is also unnecessary: `git-credential-mosaic` is invoked
|
|
by git's credential-helper protocol (stdin/stdout), which works identically under Git for
|
|
Windows' bundled `bash`/`sh` when configured via `credential.helper`, without a `.ps1`
|
|
counterpart. A `tea`-login-based per-agent identity for the PowerShell wrappers is a
|
|
separate, larger design (mapping identities to `tea login` profiles) and is out of scope
|
|
here.
|